Closed Bug 1373961 Opened 7 years ago Closed 4 months ago

Update the bookmarks menu button panel/menu to match photon styling better

Categories

(Firefox :: Theme, defect, P4)

56 Branch
Unspecified
Windows 10
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 --- wontfix
firefox57 --- affected

People

(Reporter: alice0775, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(5 files)

Attached image screenshot
There are no border when hover them
See attached screenshot
Blocks: 1372309
Thanks for all your bug-filing about the new Photon work, Alice!

The styling of the hamburger panel in the screenshot is as it should be. There shouldn't be a border. See https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/229252106 for the design.

We haven't touched the styling of the overflow panel yet, that's bug 1354086. It's a bit difficult to fix because of the different styles of items in there, especially without regressing the pre-photon appearance (though tbh, it wasn't very good before and there are known bugs with it, e.g. bug 1197624). We will try to address this in the near future.

Comparing the summary of this bug with the screenshot, I'm not sure if you see a third type of styling in other panels. My experience is that the styling of other "new style" menus (e.g. the library button, or if you move the history button or devtools button to the toolbar) should now (after bug 1370580) match that of the main harmburger menu. If there are different issues with those, please morph this bug with additional information / screenshots. If not, this is probably best off being marked as a duplicate of bug 1354086, where as said we'll try to bring the overflow panel in line with everything else.
Status: NEW → UNCONFIRMED
Ever confirmed: false
Flags: needinfo?(alice0775)
Err, sorry, didn't mean to mark this as UNCO.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(alice0775)
Attachment #8878818 - Attachment description: screenshot current hover style hamburger menu → screenshot beta55 hover style hamburger menu
See attachment 8878819 [details] and attachment 8878820 [details]
This is definitely regression. No border style on new photon menu.
(In reply to Alice0775 White from comment #7)
> See attachment 8878819 [details] and attachment 8878820 [details]
> This is definitely regression. No border style on new photon menu.

I'm afraid I don't understand. Not having a border is by design, as noted in comment #1. This includes all the menus. In that sense, it's not a regression - it's a deliberate outcome of some of the photon styling.

Can you clarify why you think the removal of the border is a bad thing?

(In reply to Alice0775 White from comment #3)
> Created attachment 8878816 [details]
> screenshot hover style bookmark

... so this is a bug. We should still also update the styling of the bookmarks menu. I can morph this bug, or I or you could file a separate one...
Flags: needinfo?(alice0775)
(In reply to :Gijs from comment #8)
> (In reply to Alice0775 White from comment #7)
> > See attachment 8878819 [details] and attachment 8878820 [details]
> > This is definitely regression. No border style on new photon menu.
> 
> I'm afraid I don't understand. Not having a border is by design, as noted in
> comment #1. This includes all the menus. In that sense, it's not a
> regression - it's a deliberate outcome of some of the photon styling.
> 
> Can you clarify why you think the removal of the border is a bad thing?

If it is by design, then I have no objection.
However, I think that hover background color to have a little more contrast.(at least hover background color like toolbar button)


> 
> (In reply to Alice0775 White from comment #3)
> > Created attachment 8878816 [details]
> > screenshot hover style bookmark
> 
> ... so this is a bug. We should still also update the styling of the
> bookmarks menu. I can morph this bug, or I or you could file a separate
> one...

morphing is ok
Flags: needinfo?(alice0775)
(In reply to Alice0775 White from comment #9)
> If it is by design, then I have no objection.
> However, I think that hover background color to have a little more
> contrast.(at least hover background color like toolbar button)

Bryan/Aaron, thoughts on this? (Should probably be a separate bug if we pursue this.)
Flags: needinfo?(bbell)
See Also: → 1354086
Summary: Hover style of menu item is different between hamburger menu and others → Update hover style of bookmarks menu button menu
Whiteboard: [photon-structure][triage]
Blocks: photon-structure
No longer blocks: 1372309
Summary: Update hover style of bookmarks menu button menu → Update hover/active style of items in the bookmarks menu button panel/menu
Flags: qe-verify?
Priority: -- → P2
Whiteboard: [photon-structure][triage] → [photon-structure]
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
Blocks: 1387512
Realistically, I don't think this needs to be MVP.
Priority: P2 → P3
Whiteboard: [photon-structure] → [reserve-photon-structure]
Priority: P3 → P4
I'm ending up fixing most of this in bug 1374815 anyway (wheee!).
Blocks: 1374815
Flags: needinfo?(bbell)
Some of the updates to the bookmarks menu were done in bug 1374815, but there are at least the following remaining items (and maybe more):

- the offset of submenus on Windows is not quite right. This was already broken before bug 1374815, and getting it perfectly right proved tricky, so I didn't try to fix it there (given that the other bug wasn't really about the bookmarks menu to begin with).
- the icons of the fixed items should maybe use the photon monochrome versions, taking care that we use the right foreground colour
- potentially, we could try to change the arrows to match the rest of photon. I don't know if we should, because items work differently in photon menus vs. in the native menus (open on click vs. open on hover) and so I think keeping the old arrow style would make sense. Bryan, thoughts?
Flags: needinfo?(bbell)
Summary: Update hover/active style of items in the bookmarks menu button panel/menu → Update the bookmarks menu button panel/menu to match photon styling better
Since I reported the latest duplicate bug let me explain why I vote for also changing the arrow style. I hope it's okay. :)

I think the bookmarks menu looks really out of place if it remains the only menu with the old style/behaviour and all other menus work differently. Consistency within the product is an important aspect of product design and my impression was that it's also an important aspect of Photon. But even if the different menu behaviour should be kept then the visual differences should be minimized because more differences = less consistency. If the behaviour is the only difference it's still better than a completely different menu.

I don't think "items work differently in photon menus vs. in the native menus (open on click vs. open on hover)" is a reason to keep a different arrow style because it's not clear at all from the style of the arrows how these menus work. It's the same shape, only a different style. User can't tell the difference of these menus because the arrow style is different. That's why I think there should be only one arrow style.
Severity: trivial → S4
Status: NEW → RESOLVED
Closed: 4 months ago
Flags: needinfo?(bbell)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: