Closed Bug 1893214 Opened 2 years ago Closed 2 years ago

Investigate new sidebar placement next to bookmarks toolbar

Categories

(Firefox :: Sidebar, task, P1)

task

Tracking

()

RESOLVED FIXED

People

(Reporter: sclements, Assigned: sfoster, NeedInfo)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fidefe-sidebar])

Attachments

(2 files, 3 obsolete files)

The requirements for this is to have the sidebar (both collapsed and expanded state) be placed to the left of the bookmarks toolbar if the sidebar is positioned to the left (there's a pre-existing "sidebar.position_start" pref). Spec is here.

If it's positioned to the right, the sidebar should sit below the bookmarks toolbar, per the spec here.

However, Dão has pointed out this warrants investigation due to how the toolbar is placed within the toolbox. Full context from his comment is here.

If we can't make these changes in a performant way or without undue complexity, then we'll need to go back to UX/product with what we can do.

hrm, looking at this spec closer and talking to UX, it seems when the sidebar panel opens it will also push the toolbar. This seems problematic in that both the sidebar and panel will be resizeable.

Assignee: nobody → sfoster

My initial thoughts: probably doable but fiddly. If we move the toolbar#PersonalToolbar element out of toolbox#navigator-toolbox and into hbox#browser as a sibling to vbox#appcontent, we get the structure we need. That would let us show the sidebar on either side as required. But, we have quite a few places where we assume that all these toolbars are children of #navigator-toolbox. This comes into play with the customize panel, with drag/dropping items, with full screen mode and possibly others I've not found yet. We also draw a bottom border on #navigator-toolbox as the separator between the brower toolbars and the content area. That will need to move - possibly to a top border on #tabbrowser-tabbox, but there are likely knock-on effects there as well.

dao, if you have a minute can I get your thoughts on this approach? There's a lot of detail obviously missing from the patch, but fundamentally, I'm moving the bookmarks toolbar out of the #navigator-toolbox and into the #appcontent element. As far as I can see 'appcontent' is just another layout container (despite the implication of the name), and while I need to adjust some CSS, some queries and assumptions that all toolbars are descendants of #navigator-toolbox this should otherwise work well.

Flags: needinfo?(dao+bmo)

If we do it this way - moving the toolbar - it would ideally be done in the markup. Its technically possible to check the pref and move it at runtime at startup or when the window opens, but that seems potentially problematic. It would be a full layout flush, and I'm not sure what other races we might get into switching up the DOM in that way.

Attached image banner overlap.png

I took your patch for a test drive and if we pursued this approach, we'd have to figure out a solution for the overlapping of notification banners.

(In reply to Sarah Clements [:sclements] from comment #7)

I took your patch for a test drive and if we pursued this approach, we'd have to figure out a solution for the overlapping of notification banners.

Yes, we'll need to shuffle some z-indexes there I think.

To summarize, the approach in attachment 9400611 [details] bring the sidebar into alignment with the bookmarks toolbar by moving that toolbar out of its current container and to become a sibling to the sidebar where we can use flexbox to give us left or right alignment. In a lot of ways that works well, but we lose the theme background images behind the bookmarks toolbar.

In attachment 9401548 [details], the DOM structure is unchanged and we just use CSS to move the sidebar up into the bookmarks toolbars space. That works ok, we just need to keep track of the sidebar width to push over the bookmarks toolbar contents so they aren't obscured by the sidebar which occupies some of that space. The demo made clear a problem here: as the sidebar can occupy up to 75% of the window width, we end up with a very cramped bookmarks toolbar.

attachment 9402354 [details] shifts the sidebar up a bit to remove the border between it and the horizontal toolbars. It also applies the theme background image styles and toolbar colors to the sidebar to demonstrate how this looks great in some themes and is pretty broken in others.

This screenshot is with Alpenglow applied - which has a transparent toolbar color and a background image which does not expect to be repeated in the sidebar.

Just to summarize the findings here:

  • We decided to leave the bookmarks toolbar full-width for now, and not move the sidebar ("launcher") up into that space.
  • We do want to remove the visual divider between the #navigator-toolbox and the sidebar, allowing that to look seamless where the theme allows for it. In some cases we can simply use the frame/frame_inactive colors for the sidebar launcher, with the toolbar color on top of that. (Note: not the sidebar color: that will remain in use for the sidebar panels and not for the launcher part which we are treating more like a toolbar)
  • However, many (~75%) themes are built with one or more background images, and there isn't a practical way at this point to extend or seamlessly tile those images to provide the background for the sidebar.

To understand the impact of this and help contextualize decisions here, we looked at manifests for all themes available at addons.mozilla.org with over 1000 users/installs

  • Themes which define a translucent toolbar (not opaque or fully transparent): 21%
  • Themes which define a fully opaque toolbar color: 35%
  • Themes which define either theme_frame or additional_images: 78%
  • Themes which define a non-opaque toolbar color and background image(s): 20.7%

So, while some themes define an opaque toolbar color (see mdn docs on the manifest theme colors), most are using a background image. And of those most almost all are using the toolbar color as a translucent overlay. This will need to feed into how we implement the logic for how we derive the sidebar launcher's color from the theme's colors.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Attachment #9400611 - Attachment is obsolete: true
Attachment #9401548 - Attachment is obsolete: true
Attachment #9402354 - Attachment is obsolete: true
See Also: → 1952602
See Also: → 1913510
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: