Revamped sidebar doesn't play well with macOS toolbar slide-down animation during full-screen
Categories
(Firefox :: Sidebar, defect, P2)
Tracking
()
People
(Reporter: emilio, Assigned: nsharpley)
References
Details
(Whiteboard: [fidefe-sidebar])
Attachments
(6 files)
Screenshot
Right now it's even more broken due to bug 1921811, but even before that regression it looks really odd, because we shift the toolbox down but not the sidebar, see screenshot.
|
Reporter | |
Comment 1•10 days ago
|
||
STR is:
- sidebar.revamp = true
- Go into macOS full-screen (click green button in the titlebar)
- Move the mouse to the top of the screen.
|
|
Reporter | |
Comment 2•10 days ago
|
||
Apparently on latest macOS the expectation is that the buttons remain taking space, and there is extra shadow around the header while the menubar is shown... Should we do something like that? It'd certainly make stuff simpler... Maybe
|
|
Reporter | |
Comment 3•10 days ago
|
||
Apparently both chrome and safari do that kind of "titlebar buttons show up without shifting everything".
|
|
Reporter | |
Comment 4•10 days ago
|
||
(Even on sonoma)
|
|
Reporter | |
Updated•10 days ago
|
|
||
Updated•10 days ago
|
|
||
Updated•10 days ago
|
|
||
Comment 5•10 days ago
|
||
Sam might also have some thoughts on this, since he worked on bug 1899598 which hasn't landed yet.
|
|
Reporter | |
Comment 6•9 days ago
|
||
Yeah I ni?d sam in bug 1921811 which is fairly related.
|
||
Updated•9 days ago
|
|
|
||
Comment 7•9 days ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)
Apparently both chrome and safari do that kind of "titlebar buttons show up without shifting everything".
Oh, it's interesting that Chrome doesn't have the extra titlebar here. I was under the assumption that the only way to get rid of the extra bar would be to have an NSToolbar in the window, and to be able to slide the toolbar contents over to the right to make space for the window buttons. It seems Chrome found a way to make things look better without having an NSToolbar. But they reserve space for the buttons and don't slide anything to the right, unlike Safari.
Oh, and native apps with sidebars always have a full-height sidebar, and then it's that sidebar space that is the reserved space for the toolbar buttons.
I don't have a strong opinion on what to do in Firefox. But if we want to get rid of the slide-down-empty-titlebar, that'll require some widget work.
|
|
||
Comment 8•9 days ago
|
||
It looks like Chrome is still tweaking what they're doing. I found https://chromium-review.googlesource.com/c/chromium/src/+/5837702 from yesterday.
|
|
||
Updated•8 days ago
|
|
||
Updated•8 days ago
|
|
||
Comment 10•8 days ago
|
||
I'm not sure I have much to add here. The proposed plan in bug 1921811 to re-cast the url bar results container as a popover seemed reasonable to me, and IIUC that would open up new options for this bug?
As far as bug 1899598 goes, the important thing that changes is that the first visible toolbar can function as the titlebar, and when you hide the tabstrip toolbar, that becomes the #nav-bar. The z-index thing came up there as when the #nav-bar is a titlebar, it gets a opacity property when the window its in becomes inactive. That means we get a new stacking context, which means the URL bar results element within would get clipped. We didn't have this problem before because the only popup/z-indexed elements you could create from the tabstrip toolbar were menus and tooltips.
|
||
Comment 11•2 days ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
Apparently on latest macOS
Please can we clarify which version this relates to? (Is it Sequoia?) And does this problem only occur there, or also on earlier versions?
the expectation is that the buttons remain taking space, and there is extra shadow around the header while the menubar is shown... Should we do something like that? It'd certainly make stuff simpler... Maybe
I'm sorry but I can't follow. Which buttons/shadow/header is this talking about? :-)
A screencast would probably help.
Based on the conversation here it's not entirely clear to me if this needs a widget/cocoa fix or if there is something for frontend to do here - clearing that up would also be helpful. Apologies if I've missed other conversations about this outside of this bug with Nikki who is now assigned to this?
|
|
Reporter | |
Comment 12•2 days ago
|
||
(In reply to :Gijs (he/him) from comment #11)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
Apparently on latest macOS
Please can we clarify which version this relates to? (Is it Sequoia?) And does this problem only occur there, or also on earlier versions?
This problem happens on any macOS version, because we do the same "shift the toolbar down" animation everywhere.
On older macOS version that was the usual thing to do (IIUC). It seems at least on Sequoia (but Chrome at least also does it on older macOS versions) the expectation is that the titlebar keeps taking space, and the buttons either fade in or shift in. Which would solve this.
I'm sorry but I can't follow. Which buttons/shadow/header is this talking about? :-)
This is the window buttons (as in, the semaphore buttons for close / minimize / etc).
A screencast would probably help.
Will attach some screencasts (of Sequoia only because that's the only thing I have available, but someone checked that Chrome at least does the same on Sonoma).
Based on the conversation here it's not entirely clear to me if this needs a widget/cocoa fix or if there is something for frontend to do here - clearing that up would also be helpful. Apologies if I've missed other conversations about this outside of this bug with Nikki who is now assigned to this?
This would need some widget/cocoa work at least (to prevent the empty titlebar from showing in), see comment 7. Not sure how trivial to do that would be, but it seems we can probably save some time by looking at what Chromium is doing.
|
|
Reporter | |
Comment 13•2 days ago
|
||
|
|
Reporter | |
Comment 14•2 days ago
|
||
|
|
Reporter | |
Comment 15•2 days ago
|
||
|
|
Reporter | |
Comment 16•2 days ago
|
||
|
|
Reporter | |
Comment 17•2 days ago
|
||
Do the screencasts above help?
|
|
||
Comment 18•1 day ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)
Do the screencasts above help?
Yes. But I confess I'm still a bit lost.
In particular, ISTM that there is a "simple" fix here which is to also adjust the top coordinate of the sidebar (ie make that shift down when the animation happens), which doesn't need any platform support and would keep parity with the current animation, and would (IMO) look non-terrible, even if it's not quite the "perfect" solution from a macOS integration PoV for Sequoia.
Then there is updating all of the behaviour for Sequoia, which would also solve the sidebar issue (because the navbar would not move) but needs platform support. Given the need for platform support I'd expect that we may not be able to apply this fix on earlier versions of macOS (depending on when the relevant APIs are available).
Is that right? Am I missing something (e.g. the first fix being much harder than I'm making out and/or the second one being easier + more generally applicable)? If not, would it make sense to split these two efforts off so they are 2 bugs instead of 1? :-)
|
Assignee | |
Comment 19•1 day ago
•
|
||
I think Emilio covered most of it! But I do think :Gijs 's suggestion to just translate the sidebar down warrants some more investigation. Before we look into changing the navbar to behave more like chrome or safari on fullscreen macs, I'll see if I can prototype something here. Stay tuned.
|
|
Reporter | |
Comment 20•1 day ago
|
||
(In reply to :Gijs (he/him) from comment #18)
Is that right? Am I missing something (e.g. the first fix being much harder than I'm making out and/or the second one being easier + more generally applicable)? If not, would it make sense to split these two efforts off so they are 2 bugs instead of 1? :-)
Yes, the first fix happens to be very tricky because with sidebar.revamp=true, since on that configuration the content separator (and shadow) is drawn from the tabbox. Which means that the tabbox should paint on top of the toolbox and sidebar.
So keeping doing what we're doing happens to be somewhat tricky because it either breaks the content separator (which is currently broken in nightly already), or it makes content overlap on top of the toolbox (which would be undesirable).
Another alternative is reconsidering the shadow separator to make it part of the sidebar and toolbox instead, but I don't see how to make that work easily either.
|
|
Reporter | |
Comment 21•1 day ago
|
||
I might be missing something, please correct me if that's the case :)
|
|
Assignee | |
Comment 22•1 day ago
|
||
This is a WIP. It unclear if it would affect performance using this method.
|
|
Assignee | |
Comment 23•1 day ago
|
||
:emilio, I'm not sure if the WIP patch in comment 22 would affect performance, but this is what it would look like if we only shift the sidebar down. It does look a little funky because we lose the top shadow for the tabbrowser-tabbox, but perhaps that's solveable with adding a bottom shadow to the navigator-toolbox upon translate, similar to Chrome and Safari? I'll attach a screenshare.
|
|
||
Comment 24•1 day ago
|
||
(In reply to Nikki Sharpley (:nikkis) (she/her) from comment #19)
I think Emilio covered most of it! But I do think :Gijs 's suggestion to just translate the sidebar down warrants some more investigation. Before we look into changing the navbar to behave more like chrome or safari on fullscreen macs, I'll see if I can prototype something here. Stay tuned.
Emilio pointed out to me (which I hadn't realized, sorry) that the border/overlap problem also applies to the main web content, not just the sidebar - right now the border between web content and the toolbar is generated from https://searchfox.org/mozilla-central/rev/7a85a111b5f42cdc07f438e36f9597c4c6dc1d48/browser/themes/shared/tabbrowser/content-area.css#51-55 . It's not visible on Nightly atm but it becomes more obvious if you apply Nikki's patch in https://phabricator.services.mozilla.com/D224201#change-OayWqRyT5dTQ .
So if we move the toolbox down, it ends up "on top of" that and we lose the border. Moving the sidebar down solves the sidebar but not the web content side of things.
We can generate a border on the entirety of the moving nav toolbox but then that border overlaps the sidebar which looks odd if the sidebar also moves.
Looking at other macOS apps on Sonoma, oddly Calendar.app already does the same thing as Chrome on Sequoia: it fades/moves the titlebar buttons inside the main toolbar.
On the other hand, Music.app overlaps its own sidebar with the toolbar content and has a shadow along the entire edge (also on top of the sidebar).
So I see at least the following 3 options:
- move everything. Nobody else does this and it's kind of my lazy idea but hear me out: this is a 1-line patch on top of https://phabricator.services.mozilla.com/D224201 (just put the translate on
gNavToolbox.parentElementinstead ofgNavToolbox). Then web content, the sidebar, the toolbox - everything moves to make space for the titlebar. And sure, it's kinda weird - but the whole point of this interaction is that the user wants the menubar / titlebar. AFAIK there's no way to "persist" with this when not hovering the top of the window, so presumably it's not actually bad that some number of pixels get cut off at the bottom of the screen. - move the toolbox as we do now and swap over the bottom border to be a shadow along the toolbox, which overlaps web content and the sidebar (a la Music.app). This is more work but should be doable: we'd put shadow CSS back on the
gNavToolboxfor this situation based on some selector and then that would show the overlap on top of the sidebar and web content. Not ideal but matches Music.app so not awful, other people did this first. This would also match the non-sidebar-revamp state, where the toolbox already overlaps the sidebar. - do the Sequoia / Calendar.app thing and fade the titlebar buttons in/out instead of any vertical translation. This would require some work on the widget side in order to apply opacity to the titlebar buttons, and to not have the animating titlebar on top of us, but other than that we could potentially do this in frontend. Another wrinkle would be toolbar overflow in this situation, and making the animation smooth. So this definitely seems the most complex.
I would suggest settling on (1) or (2) for 133 and have (3) as some kind of stretch goal; it's not clear to me how nice it'd be in the toolbar overflow situation. Calendar.app doesn't have this problem and we do...
|
|
||
Comment 25•1 day ago
|
||
(argh, sorry - I hadn't seen the earlier patch and comment from Nikki before posting my essay...)
|
|
Assignee | |
Comment 26•1 day ago
|
||
:Gijs, you put it more eloquently! :)
My patch is sort of a spin on option 1. I like the idea of making everything shift down, rather than just the sidebar as I've done, but I think we'd need to change the height of the browser element too, no? Option 2 was my other thought as well! I don't think it would be a considerable amount of effort to add the box-shadow on the gNavToolbox (famous last words!)
Agreed, 3 involves far more work. We can come back to it, but either 1 or 2 is best for now.
I guess the question is, who makes the call? Do we need to loop someone from UX in?
|
|
Reporter | |
Comment 27•1 day ago
|
||
We shouldn't change heights during the animation, that'd make it jank quite a bit.
|
|
Reporter | |
Comment 28•1 day ago
|
||
(In reply to :Gijs (he/him) from comment #24)
Another wrinkle would be toolbar overflow in this situation, and making the animation smooth. So this definitely seems the most complex.
I'm not sure I get this, what's the issue with toolbar overflow / making the animation smooth? If we fade-in the titlebar buttons from widget code, I think this is literally zero work for the front-end? Or am I missing something? I think it's worth at least trying this out, fwiw, I don't think it'll end up being that much widget work.
Were you talking about a potential slide-in animation or so? If so I'd agree that seems like more work. But I don't think that's required to avoid the shift.
|
|
||
Comment 29•18 hours ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #28)
(In reply to :Gijs (he/him) from comment #24)
Another wrinkle would be toolbar overflow in this situation, and making the animation smooth. So this definitely seems the most complex.
I'm not sure I get this, what's the issue with toolbar overflow / making the animation smooth? If we fade-in the titlebar buttons from widget code, I think this is literally zero work for the front-end?
Well, if we always have the buttons take up space, we'd need to always include the titlebar buttons in the navbar in fullscreen, and apply some opacity and visibility CSS to fade in/out depending on menubar/titlebar hover state etc., so not sure I agree with "literally zero", but it's certainly not a lot of work, comparatively speaking, if we can update the widget code to honour opacity on these buttons. (I think we need visibility as well so that the buttons don't "work" while hidden via opacity, assuming that works for the native buttons...)
But also, in this proposal AIUI the buttons always take up space so that kind of begs the question of why we wouldn't always show them rather than showing an empty space until you hover the menubar? What's the point of the empty space?
Were you talking about a potential slide-in animation or so? If so I'd agree that seems like more work. But I don't think that's required to avoid the shift.
I was talking about having the buttons not take up space when not hovering the toolbox/titlebar, and then take up space once the menubar is hovered until the mouse leaves the navbar+menubar+titlebar again. And yes, Calendar.app also has a slide-in/out animation between those two states. That (with or without a slide-in animation) would change the amount of space available for everything else in the navbar and thus trigger toolbar overflow/underflow, potentially moving widgets into the overflow panel etc. Which IMO would look messy - the opacity fade I'm sure could be smooth but moving the widgets isn't...
Does that help clarify? :-)
|
|
Reporter | |
Comment 30•16 hours ago
|
||
But also, in this proposal AIUI the buttons always take up space so that kind of begs the question of why we wouldn't always show them rather than showing an empty space until you hover the menubar? What's the point of the empty space?
Mostly consistency with the other apps. But sure we could also just do that (suppress the current effect, always show the buttons), then figure out if we want something fancier (either an opacity animation, or an slide-in animation) in a follow-up.
I was talking about having the buttons not take up space when not hovering the toolbox/titlebar, and then take up space once the menubar is hovered until the mouse leaves the navbar+menubar+titlebar again.
Yeah, that seems like a bit of a pain to do smoothly.
Does that help clarify? :-)
Yes, thanks!
I guess the main question is which path forward to go:
- Keep the current effect, slide in the whole
<body>(comment 24): Somewhat trivial, but not what any other apps do. - Keep the current effect, rejigger stuff to add border / shadow to the toolbox while animating only (keep overlapping the sidebar).
- Remove the current shift altogether (unfix bug 1699506): Also trivial of course. Maybe worth considering? This gets us the Music.app behavior on my machine.
- Suppress the current effect, always show the titlebar buttons in fullscreen (I think this is what chrome plans to do in the link for comment 8).
- Suppress the current effect, do an opacity animation to fade them in (comment 14).
- Suppress the current effect, do a more complex animation to slide them in (comment 15,): As noted above, quite a few technical challenges.
I think we should discard (6) altogether for now at least for the reasons you mentioned.
Should we ask UX? My gut feeling is that in terms of complexity (simplest to hardest) we have 3, 1, 4, 5, 2 (where 4 and 5 need some widget work).
In terms of personal preference (in terms of the effort to get it working vs. maintainability) I think I'd go: 4/5, 1, 2, 3. But I don't feel too strongly.
|
|
||
Updated•9 hours ago
|
|
|
||
Comment 31•9 hours ago
|
||
I'm marking as P2. I think this still needs improvement but is workable for now.
Description
•