Closed Bug 1737361 (sn-utb-rebuild) Opened 3 years ago Closed 1 year ago

[meta] Rebuild the toolbar to only have one dynamic implementation

Categories

(Thunderbird :: Toolbars and Tabs, task, P2)

Tracking

(Not tracked)

RESOLVED FIXED
115 Branch

People

(Reporter: aleca, Assigned: freaktechnik)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: meta)

Attachments

(1 obsolete file)

I'm not sure if this is technically doable, but maybe it's something worth exploring.

Rebuild our toolbar in order to have only one dynamic implementation
Currently, we have a dedicated toolbar for each tab. This is kinda necessary because each tab has different toolbarbuttons and different needs.

This approach falls short once we start needing to implement the same button on each toolbar. The most obvious example is the main app menu button, which we need to initialize multiple times for each toolbar location.

It also turns tedious when new buttons are implemented that need to be available in every toolbar or when new tabs are implemented (new address book missing the app menu button).

Proposal
We should have only one toolbar.
We should then find a decent mechanism that allows us to:

  • Have all the available <toolbarbutton>s defined in one single file.
  • Independently define a defaultset of toolbarbuttons available per tab.
  • Independently define a set of all customizable toolbarbuttons available per tab.

What do you guys think?

That would be great as we then can move the toolbar out of the tabmail-container into the navigation-toolbox.

Recently when investigating if I could figure out how to make the chat tab button not disabled in non-3pane tabs I came across the command controllers (for example https://searchfox.org/comm-central/source/mail/base/content/mail3PaneWindowCommands.js). these seem to already dynamically enable and disable commands depending on the tab. Whatever we do should probably at least be related to this, given that toolbarbuttons are often using commands as their handler.

Blocks: 1721896

I'm gonna take this and make it happen after bug 1736437 landed, since we need to have only 1 toolbar if we want to keep having proper background image styling for LW-Themes.

Assignee: nobody → alessandro
Depends on: 1736437
Priority: -- → P2

I'm thinking we should have some kind of registry service (non-xpcom) that keeps watch over the toolbar and changes the configuration on behalf of the respective areas instead of them implementing it themselves. The buttons could be generated here from lightweight configuration objects that are stored in a Map so we can easily switch between them by name.

I've been thinking a bit about this during the break and this is the conclusion I reached.

What's needed
A way to let each tab view define:

  • A list of toolbar buttons.
  • A list of currentset and defaultset to keep a working customization.
  • A way to "use" an existing buttons set from another view.

The almost sure approach for this is to rely on the TabMonitor to react to tab changes.
On every tab change, we can see if the current tab needs a toolbar and which buttons we should show, and if needed we can update the UI to reflect that.

Using a <slot> element as general template
The general idea would see the introduction of a <slot> element, mimicking the VueJS Slots components https://vuejs.org/v2/guide/components-slots.html, which was inspired by the Web Components spec draft https://github.com/WICG/webcomponents/blob/gh-pages/proposals/Slots-Proposal.md.

The <slot> component would act kind of like a template, so not rendered in the page but used by the onTabSwitched() method to check if the current tab has it and what's inside.
This approach would allow us to write those toolbarbuttons directly in the XHTML, which maybe it makes it easier to maintain(?).

We could also introduce a data-attribute that can allow us to use a the same set of buttons from another <slot> by referencing its ID, in case multiple tabs have the same configuration.

A possible example could be something like this:

// Mail tab
<slot id="mail-toolbar" defaultset="...">
  <toolbarbutton/>
  <toolbarbutton/>
  <toolbarbutton/>
  ...
</slot>

// Calendar tab
<slot id="calendar-toolbar" defaultset="...">
  <toolbarbutton/>
  <toolbarbutton/>
  <toolbarbutton/>
  ...
</slot>

// Tasks tab
<slot data-slot="calendar-toolbar"/>

With this approach we can have:

  • Each <slot> element living inside the tab they belong to.
  • For those tabs that don't need a toolbar (preferences, account managers, setup, etc), we simply don't add a <slot>.
  • JS will only handle clearing and repopulating the main toolbar when a tab is changed.
  • We can still leverage the defaultset and currentset to maintain customization and store those changes in the XUL Storage.

Thoughts?

I'd think the toolbar belongs (is owned by) the tab though. Especially after Geoff's new 3pane work.
So there's no clearing/repopulating a main toolbar: the tab needs to render a toolbar the way it wants it, if it wants it.

(In reply to Magnus Melin [:mkmelin] from comment #6)

I'd think the toolbar belongs (is owned by) the tab though. Especially after Geoff's new 3pane work.
So there's no clearing/repopulating a main toolbar: the tab needs to render a toolbar the way it wants it, if it wants it.

I don't agree. While it seems nice conceptually it's not practical. It would require an implementation for every tab type with all of the problems that brings such as making the same change in multiple places. Adding a toolbar to the new Address Book was a nightmare and yet it still doesn't have some of the things it should have, like the app menu. Nor is it customisable which might not matter for the Address Book but will for other tab types.

We already have one menu bar for every tab, and nobody's suggesting that it be owned by the tabs.

Assignee: alessandro → nobody
Summary: Rebuild the toolbar to only have one dynamic implementation → [meta] Rebuild the toolbar to only have one dynamic implementation
Assignee: nobody → martin
Status: NEW → ASSIGNED
Depends on: 1784289

Comment on attachment 9289464 [details]
Bug 1737361 - Add unified toolbar to messenger window. r=aleca

Revision D154397 was moved to bug 1784289. Setting attachment 9289464 [details] to obsolete.

Attachment #9289464 - Attachment is obsolete: true
Depends on: 1786042
Depends on: 1788630
Depends on: 1788640
Depends on: 1788641
Depends on: 1788649
Depends on: 1747291
No longer depends on: 1788649
Depends on: 1789653
Depends on: 1795278
Depends on: 1795280
Depends on: 1795360
Depends on: 1798509
Depends on: 1799669
Depends on: 1803567
Depends on: 1803568
Depends on: 1803569
Depends on: 1803570
Depends on: 1803571
Depends on: 1803582
Depends on: 1803857
Depends on: 1805890
Depends on: 1805902
Depends on: 1805912
Depends on: 1806376
Regressions: 1806586
Depends on: 1809312
Depends on: 1809896
Depends on: 1810512
Depends on: 1810521
Depends on: 1810713
Depends on: 1810751
See Also: → sn-unifiedtoolbar
Depends on: 1811838
Depends on: 1812292
Blocks: 1812968
Depends on: 1813490
Depends on: 1814370
Depends on: 1814470
Depends on: 1814664
Depends on: 1815210
Depends on: 1816435
Depends on: 1816913
Alias: sn-utb-rebuild
Keywords: meta
Depends on: 1817947
Depends on: 1817948
Depends on: 1817949

All ongoing work is tracked in meta bug 1811380

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Depends on: 1841489
No longer depends on: 1841489
Target Milestone: --- → 115 Branch
Regressions: 1872239
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: