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

NEW
Unassigned

Status

()

P4
trivial
a year ago
11 months ago

People

(Reporter: alice0775, Unassigned, NeedInfo)

Tracking

(Blocks: 2 bugs)

56 Branch
Unspecified
Windows 10
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 wontfix, firefox57 affected)

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(5 attachments)

(Reporter)

Description

a year ago
Created attachment 8878800 [details]
screenshot

There are no border when hover them
See attached screenshot
(Reporter)

Updated

a year ago
status-firefox54: --- → unaffected
status-firefox55: --- → unaffected
status-firefox56: --- → affected
status-firefox-esr52: --- → unaffected
Component: Menus → Theme
(Reporter)

Updated

a year ago
Blocks: 1372309

Comment 1

a year ago
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)

Comment 2

a year ago
Err, sorry, didn't mean to mark this as UNCO.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 3

a year ago
Created attachment 8878816 [details]
screenshot hover style bookmark
Flags: needinfo?(alice0775)
(Reporter)

Comment 4

a year ago
Created attachment 8878818 [details]
screenshot beta55 hover style hamburger menu
(Reporter)

Comment 5

a year ago
Created attachment 8878819 [details]
screenshot hover style histoly submenu
(Reporter)

Updated

a year ago
Attachment #8878818 - Attachment description: screenshot current hover style hamburger menu → screenshot beta55 hover style hamburger menu
(Reporter)

Comment 6

a year ago
Created attachment 8878820 [details]
screenshot beta55 hover style histoly submenu
(Reporter)

Comment 7

a year ago
See attachment 8878819 [details] and attachment 8878820 [details]
This is definitely regression. No border style on new photon menu.

Comment 8

a year ago
(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)
(Reporter)

Comment 9

a year ago
(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)

Comment 10

a year ago
(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: → bug 1354086
Summary: Hover style of menu item is different between hamburger menu and others → Update hover style of bookmarks menu button menu

Updated

a year ago
Whiteboard: [photon-structure][triage]

Updated

a year ago
Blocks: 1349210
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]

Updated

a year ago
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly

Comment 11

a year ago
Realistically, I don't think this needs to be MVP.
status-firefox56: affected → wontfix
status-firefox57: --- → affected
Priority: P2 → P3
Whiteboard: [photon-structure] → [reserve-photon-structure]
Priority: P3 → P4

Comment 12

a year ago
I'm ending up fixing most of this in bug 1374815 anyway (wheee!).
Blocks: 1374815

Updated

a year ago
Flags: needinfo?(bbell)

Updated

a year ago
Duplicate of this bug: 1396382

Comment 14

a year ago
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

Comment 15

a year ago
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.
You need to log in before you can comment on or make changes to this bug.