Closed Bug 1392978 Opened 7 years ago Closed 2 years ago

New Tab button should reduce height by 2px in compact mode

Categories

(Firefox :: Theme, defect, P5)

57 Branch
x86_64
Windows 7
defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox-esr68 --- wontfix
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox67.0.1 --- wontfix
firefox68 --- wontfix
firefox69 --- wontfix

People

(Reporter: Virtual, Unassigned)

References

Details

(Keywords: nightly-community, regression, ux-consistency, Whiteboard: [reserve-photon-visual])

Attachments

(7 files, 10 obsolete files)

(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
(deleted), image/png
Details
Follow up from Bug #1389921, to make New Tab button look better. "New tab" button in Compact Density mode: - 1 px space from top - 2 px space from left - 1 px space from bottom "New tab" button in Normal Density mode: - 2 px space from top - 2 px space from left - 2 px space from bottom "New tab" button in Touch Density mode: - 3 px space from top - 2 px space from left - 3 px space from bottom So in tl;dr - New Tab button should be moved: - 1 px to left in Compact Density mode - 1 px to right in Touch Density mode - Normal Density mode should be left unchanged
Has Regression Range: --- → yes
Has STR: --- → irrelevant
Whiteboard: [photon] [triage] → [photon-visual] [triage]
Priority: -- → P4
Whiteboard: [photon-visual] [triage] → [reserve-photon-visual]
No longer blocks: photon-visual
Flags: qe-verify+
Depends on: 1398103
Attached image new tab buttons (200%).png (deleted) —
The purpose of this bug is fixed by bug 1398103 as far as I can see. However the new tab button in normal and touch mode is still a bit higher from the bottom. According to the mockup there should not be such space. See attachment. :Virtual - what are you seeing ?
Flags: needinfo?(Virtual)
(In reply to Eddward from comment #11) > Created attachment 8924722 [details] > new tab buttons (200%).png > > The purpose of this bug is fixed by bug 1398103 as far as I can see. > However the new tab button in normal and touch mode is still a bit higher > from the bottom. According to the mockup there should not be such space. > See attachment. > :Virtual - what are you seeing ? Unfortunately, this bug isn't fixed yet at all, even by patch from bug 1398103. I tested this on Mozilla Firefox Nightly 58.0a1 (2017-11-05) (64-bit) Updating comment #0, now there is: "New tab" button in Compact Density mode: - 0 px space from top - 1 px space from left - 0 px space from bottom "New tab" button in Normal Density mode: - 2 px space from top - 2 px space from left - 2 px space from bottom "New tab" button in Touch Density mode: - 3 px space from top - 2 px space from left - 3 px space from bottom So in tl;dr - still, New Tab button should be moved: - 1 px to left in Compact Density mode - Normal Density mode should be left unchanged - 1 px to right in Touch Density mode
Flags: needinfo?(Virtual)
Well, then I think the title of this bug should be modified to be clear that it is Win 7 specified. As you can see from my Win 10 screeshot there is no way to move new tab button 1px to the left in compact mode since there is only 1px gap between the tab and new tab button. In touch density there is 2px gap, the same as in mockup.
(In reply to Eddward from comment #13) > Well, then I think the title of this bug should be modified to be clear that > it is Win 7 specified. > As you can see from my Win 10 screeshot there is no way to move new tab > button 1px to the left in compact mode since there is only 1px gap between > the tab and new tab button. In touch density there is 2px gap, the same as > in mockup. This bug is for all systems, not only specified for Windows 7, as there was standardization, not that long time ago, to be Firefox GUI consistent on all platforms and on all operating systems (in most cases of course, except some odd cases probably). What's more, that this bug is reproducible with Firefox on Windows 7, it's stated in "Platform:" in bug description.
But did you test it on all systems ? The point is that I can no longer see this bug on Win 10. It's not reproducible anymore, at least for me.
Assignee: nobody → dharvey
So I dont have a window 7 box to test with yet, however on osx and windows 10 this matches the relevant specs in which they have 1px outer padding in compact mode and 2px in touch mode, provided by https://searchfox.org/mozilla-central/source/browser/themes/shared/toolbarbuttons.inc.css#29 http://design.firefox.com/people/shorlander/photon/Mockups/macOS.html http://design.firefox.com/people/shorlander/photon/Mockups/windows-10.html I will install windows 7 to test, but it seems given that as mentioned these are to be consistent across platforms thats its likely nothing to fix here
Thanks for picking up the bug. I was thinking the same in comment #11. There is nothing to fix at least on Win 10, but :Virtual is still seeing the bug on Win 7. However, can you please check if there is something you can do with the gap below New tab button ? See screenshot from comment #11. In Normal and Touch density there is some excessive gaps below and perhaps also above button which are not in the Mockup. I can file a separate bug if it is a real issue.
Attached image Compact (500%).png (deleted) —
Attachment #8900176 - Attachment is obsolete: true
Attachment #8900177 - Attachment is obsolete: true
Attachment #8900178 - Attachment is obsolete: true
Attachment #8900179 - Attachment is obsolete: true
Attachment #8900181 - Attachment is obsolete: true
Attachment #8900182 - Attachment is obsolete: true
Attachment #8900183 - Attachment is obsolete: true
Attachment #8900184 - Attachment is obsolete: true
Attachment #8900185 - Attachment is obsolete: true
Attachment #8900186 - Attachment is obsolete: true
Looks like there is some misunderstanding. I'm just only talking about visual perspective, that it will look much better (especially in Touch mode), when New Tab button will be moved: - 1 px to left in Compact Density mode and - 1 px to right in Touch Density mode, as there will be same distance in pixels from: left like it's from top and bottom. Please see attached and updated screenshots. BTW - If this isn't in spec, spec could be always changed, like many times before, to reflect these changes.
Well, I think it is misunderstanding then. You have not mentioned that it is mainly your proposal :) (In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see your comment/reply/question/etc.) from comment #20) > Created attachment 8931765 [details] > fixed Compact (500%).png Honestly, from visual perspective, I would strongly disagree with this. There must be at least 1 px gap (as it is now as per spec). Tab and New tab button glued together look simply bad.
(In reply to Eddward from comment #25) > Well, I think it is misunderstanding then. You have not mentioned that it is > mainly your proposal :) Maybe you omit this, but that was written in Comment #0 >(Virtual_ManPL [:Virtual] - wrote in comment #0) >> [...] to make New Tab button look better. [...] (In reply to Eddward from comment #25) > (In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see > your comment/reply/question/etc.) from comment #20) > > Created attachment 8931765 [details] > > fixed Compact (500%).png > > Honestly, from visual perspective, I would strongly disagree with this. > There must be at least 1 px gap (as it is now as per spec). Tab and New tab > button glued together look simply bad. It could be preference of person, but New Tab button is already glued to top and to bottom side of screen. Another proposition (probably much better, so it will be consistent with space in Normal and Touch mode) to fix this is to make New Tab button 2 px lower in Compact Mode, so New Tab button will has 1 px of space from the top, bottom and left, so there will be the same distance in pixels from: left like it's from top and bottom. With Touch mode you're probably OK, as I'm seeing. So what do you think about it now?
I don't think there is something glued. On top you have Fitts's law, on bottom it is connected to the Navigation bar, so there is a logical reason behind this in my understanding. Mockups are perfectly fine, the best will be to follow them. The last issues are described in comment #18 with comparison screenshot in comment #11. I think anything else needs to be evaluated by UX team and none of them were involved in this bug.
There is top and bottom glued in Compact mode, as it's clearly shown on attached screenshots, so there is also no reason why New Tab button can't also be glued to be consistent, especially when there is Tab border, so this space is useless and look wrong and bad, moreover this space is active as New Tab button, so it's kinda illogical, as any border and free space don't have any function in other places.
Clarified the bug title to the new proposal (I also mistook this as a bug and not a proposal) Stephen do you think this is something we should do or WONTFIX?
Flags: needinfo?(shorlander)
Summary: New Tab button should move: 1 px to left in Compact Density mode and 1 px to right in Touch Density mode → New Tab button should reduce height by 2px in compact mode
Changing priority so it orders correctly since its assigned
Priority: P4 → P1
Unassigning for parental leave
Assignee: dharvey → nobody
Status: ASSIGNED → NEW
Priority: P1 → P4
Priority: P4 → P5
Assignee: nobody → shorlander
Flags: needinfo?(shorlander)
A nice fix to have but I am marking it fix-optional to remove it from regression triage since it's already assigned and has a priority marked by the team.
Marking fix-optional for 64. We could still take a patch in 65.
Dao, is this something you could take over for Shorlander?
Flags: needinfo?(dao+bmo)
No, if I remember correctly this is assigned to Shorlander to decide what we should do here if anything at all.
Flags: needinfo?(dao+bmo)

Bulk change for all regression bugs with status-firefox67 as 'fix-optional' to be marked 'affected' for status-firefox68.

Stephen can you make a decision here?

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: stephen → nobody
Status: ASSIGNED → NEW

Redirect a needinfo that is pending on an inactive user to the triage owner.
:dao, since the bug has recent activity, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(stephen) → needinfo?(dao+bmo)
Severity: normal → S3

I think we'll just keep this as-is.

Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(dao+bmo)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: