867121 - Defect - Snapped view: Favicons too close to text in awesome screen

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Firefox Start, defect, P1)

Description

[Virgil Dicu [:virgil] [QA]]

Attachment: Snapped view

Mozilla/5.0 (Windows NT 6.2; rv:23.0) Gecko/20130429 Firefox/23.0

1. Launch nightly in Immersive Mode.
2. Put Firefox in snapped view.
3. Invoke the awesome screen.

Actual: layout issue - The text is too close to the edge of the favicon - for all items.
Easier to observe for non white background.

Comment 1

[Allison Naaktgeboren :ally]

Not a defect per se. Personal preference of the engineer who wrote it(me).

A little padding probably won't hurt anyone' eyes.

Comment 2

[Marco Mucci [:MarcoM]]

Hey Ally, what is your estimation for the point value on this defect?

Comment 3

[Allison Naaktgeboren :ally]

Well it should be 1, but there's something messed up in the css logic that I stumbled onto, so probably p=3.

Comment 4

[Allison Naaktgeboren :ally]

Attachment: add a left margin when snapped

I went with 3 px. It was arbitrary. 4 might be better?

You can trigger the snapped view on about:start by adding this to startUI's init
    // hop into snapped view (debug so I can get dom inspector)
    Elements.windowState.setAttribute("viewstate", "snapped");
    Services.obs.notifyObservers(null, "metro_viewstate_dom_snapped", null);

original rules were:
#snapped-start[viewstate="snapped"] .canSnapTiles .richgrid-item-desc,
#panel-container[viewstate="snapped"] .canSnapTiles .richgrid-item-desc

Comment 5

[Matt Brubeck (:mbrubeck)]

Comment on attachment 746143 [details] [diff] [review]
add a left margin when snapped

Review of attachment 746143 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/theme/browser.css
@@ +871,5 @@
>    -moz-box-orient: horizontal;
>  }
>  
> +[viewstate="snapped"] .canSnapTiles .richgrid-item-desc {
> +  margin-left: 3px;

Nit: Use -moz-margin-start

I'd like to change this from 3px to 8px.  That will match the padding on the opposite side of the favicon, so the favicon ends up centered within its "frame."  The mocks in bug 801000 are too small to get exact dimensions, but they show a relative large margin here.

Comment 6

[Allison Naaktgeboren :ally]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2eebc32a77c

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a2eebc32a77c

Comment 8

[Marco Mucci [:MarcoM]]

For testing and verification

Comment 9

[Kamil Jozwiak [:kjozwiak]]

Attachment: text still appearing very close to icons

Went through the following Defect and found the same issue while Testing/Verification. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-05-07-03-09-48-mozilla-central/

Went through the steps provided in Comment 0 and saw the same issue as shown in the screenshot that has been attached. Went through the following tests to make sure the issue was resolved:

- Used Snapped View on the left side of the screen - Issue still exists (looks the same as the original screenshot, text appearing very close to icons)
- Used Snapped View on the left side of the screen - Issue still exists (looks the same as the original screenshot, text appearing very close to icons)

I've attached a screenshot to illustrate the issue is still occurring. From my understanding, there should be some padding added between the text and icons, please let me know if I am looking at this incorrectly!

Comment 10

[Matt Brubeck (:mbrubeck)]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #9)
> Went through the following Defect and found the same issue while
> Testing/Verification. Used the following build:
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-05-07-03-09-48-
> mozilla-central/

That nightly was from yesterday morning, before this fix landed.  Today's nightly will contain the fix, but it is not yet finished building.

Comment 11

[Kamil Jozwiak [:kjozwiak]]

My apologies Matt! Will go through it again when I get home today

Comment 12

[Kamil Jozwiak [:kjozwiak]]

Went through the following Defect and found no issue while Testing/Verification. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-05-08-03-11-13-mozilla-central/

- Ensured that the padding appeared correctly while in "Snapped View" (both on the left and right sides)
- Ensured that links worked without any issues when selecting them in "Snapped View" (both on the left and right sides)
- Ensured that sliding Firefox Metro from "Snapped View" into "Filled View" worked without any issues (both left and right sides)
- Ensured that the "Top Sites" appeared correctly in "Snapped View"
- Ensured that the "Top Sites" appeared correctly in "Full Screen View"

Comment 13

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #8 testing without any issues. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-06-10-03-11-47-mozilla-central/

- Went through the original issue as per Comment 0 without any issues
- Went through the test cases from Comment 12 without any issues

Comment 14

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #9 testing without any issues. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-06-25-03-12-38-mozilla-central/

- Went through the original issue as per Comment 0 without any issues
- Went through the test cases from Comment 12 without any issues

Comment 15

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130819030205
Built from http://hg.mozilla.org/mozilla-central/rev/c8c9bd74cc40

WFM
Tested on windows 8 using latest nightly for iteration-12.


- Went through the original issue as per Comment 0 without any issues
- Went through the test cases from Comment 12 without any issues

Comment 16

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #13 without any issues. Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-09-01-03-02-18-mozilla-central/

- Went through the original issue from comment #0 without any issues
- Went through the test cases added in comment #12 without any issues