Closed Bug 1278984 Opened 4 years ago Closed 2 years ago

Add the font panel as an optional sidebar tab

Categories

(DevTools :: Inspector: Fonts, enhancement, P2)

enhancement

Tracking

(firefox50 affected)

RESOLVED INVALID
Tracking Status
firefox50 --- affected

People

(Reporter: jdescottes, Unassigned)

References

Details

Attachments

(3 files)

Attached image optional-font-panel.gif
The font panel was removed in Bug 1247723. Even though the panel can still be activated via about:config, it would be nice if users had an easier way to access it.

One of the comments in Bug 1247723 mentioned making the tab only accessible via the menu (which is only displayed when the tabs container has an overflow at the moment). 

I implemented a prototype based on this idea:
- sidebar tabs can now use an "optional" attribute
- a hidden optional tab is still listed in the sidebar menu
- a visible optional tab has a close icon
- clicking on the close icon of an optional tab hides it
- the sidebar menu button is now displayed if any of the tabs is optional and hidden (even if there is no overflow)

Using events emitted by the sidebar, the inspector panel can also store the visibility of the tab in user preferences.

(see the attachment)

Builds should be available at https://treeherder.mozilla.org/#/jobs?repo=try&revision=5219c8c92764 shortly.
Helen, what do you think about this? Even beyond the scope of the font panel, this could be a useful mechanism for handling additional/less-used panels.
Severity: normal → enhancement
Flags: needinfo?(hholmes)
(In reply to Julian Descottes [:jdescottes] from comment #1)
> Helen, what do you think about this? Even beyond the scope of the font
> panel, this could be a useful mechanism for handling additional/less-used
> panels.

I'd be all for this—the comments in bug 1247723 have been haunting me in my sleep. I agree this is also a good way to handle the less-used panels.

Wow, awesome job already having a build up!
Flags: needinfo?(hholmes)
See Also: → 1247723
Heads up that the sidebar component is being reimplemented / converted to HTML in Bug 1259819, so anything added here will need to be added in the new version too.
See Also: → 1259819
@Julian Descottes Thanks!
Implementation of optional tabs using the XUL version of the sidebar.

Review commit: https://reviewboard.mozilla.org/r/59204/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59204/
For reference, uploaded the latest version of my patch here, using the XUL sidebar widget. 
As discussed with Honza, the HTML sidebar (Bug 1259819) should land in the coming weeks, I will then migrate this patch to the HTML version of the sidebar widget. No reason to land a XUL version in the meantime. 
(behavior is still as described on comment #1)

Other options/modifications discussed:
- keep the menu button always visible
- make all the tabs optional
Depends on: 1259819
Hi Julian, should this bug be part of Track #1?
Flags: needinfo?(jdescottes)
IMO we don't have to track it in the devtools-html project. It is just an additional feature, and it makes more sense to wait for the HTML implementation of the sidebar to avoid implementing it twice. If we want to track it, that would be part of track #2 though, same as the blocking bug.
Flags: needinfo?(jdescottes)
Inspector bug triage (filter on CLIMBING SHOES).
Priority: -- → P2
I have landed bug 1259819, converted Inspector side bar into HTML, and I'd like continue with converting the all-tabs menu (bug 1281789). I have been looking at this bug and I think we could simplify the UX here.

What about:
* The all tabs menu is available all the time.
* Every tab has corresponding item in the menu.
* The user can check/uncheck any item to show/hide any tab.
* There are no special 'optional' tabs (the user can show/hide any tab)
* The `Fonts` tab can be hidden/unchecked by default

This UX is consistent e.g. with how the Storage panel columns customization UX works (and Net panel should work the same).

Honza
Flags: needinfo?(jdescottes)
Flags: needinfo?(hholmes)
(In reply to Jan Honza Odvarko [:Honza] from comment #10)
> I have landed bug 1259819, converted Inspector side bar into HTML, and I'd
> like continue with converting the all-tabs menu (bug 1281789). I have been
> looking at this bug and I think we could simplify the UX here.
> 
> What about:
> * The all tabs menu is available all the time.
> * Every tab has corresponding item in the menu.
> * The user can check/uncheck any item to show/hide any tab.
> * There are no special 'optional' tabs (the user can show/hide any tab)
> * The `Fonts` tab can be hidden/unchecked by default
> 
> This UX is consistent e.g. with how the Storage panel columns customization
> UX works (and Net panel should work the same).
> 
> Honza

Sounds good except for one point :

> * The user can check/uncheck any item to show/hide any tab.

There is one issue with this. The all-tabs-menu is supposed to allow you to select tabs that disappeared because of the overflow. Clicking a menu item here means "select this tab that I can't select otherwise". If clicking an item now toggles its visibility, how can a user select a tab that is configured to be displayed, but is hidden for overflow reasons?
The all-tabs-menu is primarily meant for navigation, not for configuration.

In my opinion, closing/hiding a tab has to remain separate from the menu.
Flags: needinfo?(jdescottes)
(In reply to Julian Descottes [:jdescottes] from comment #11)
> In my opinion, closing/hiding a tab has to remain separate from the menu.
Agreed. There might be a way to merge them though: at the bottom of the menu, have an "edit" item that, once clicked, puts checkboxes next to each menu item (also revealing the items that were hidden so far). So you can switch the menu to edit mode and change which item are shown in it.
Once an item is set to be visible in the menu, its corresponding tab appears in the sidebar.
Attached image menu.png
(In reply to Julian Descottes [:jdescottes] from comment #11)
> (In reply to Jan Honza Odvarko [:Honza] from comment #10)
> Sounds good except for one point :
> 
> > * The user can check/uncheck any item to show/hide any tab.
Yes, you are right.

(In reply to Patrick Brosset <:pbro> from comment #12)
> (In reply to Julian Descottes [:jdescottes] from comment #11)
> > In my opinion, closing/hiding a tab has to remain separate from the menu.
> Agreed. There might be a way to merge them though: at the bottom of the
> menu, have an "edit" item that, once clicked, puts checkboxes next to each
I like that.

I have also talked to Joe and he pointed out similar UI/UX in iTunes.

See the attached screenshot
- the image on the left shows a context menu with list of tabs
- it has 'Edit Menu...' item at the bottom
- when the user picks the 'Edit Menu...' there is additional popup allowing the user to pick tabs.

So, the UI is split into two parts. I. part allowing to select tabs and the II. part allowing to show/hide tabs.

If I understand correctly, Patrick's suggestion is about merging those two parts. I don't have a strong preference, I like both, except that split the logic into two parts might be simpler to implement (especially on top of the current Menu API we have available).

Honza
I like the "edit menu" option, opening a second menu as shown in your itunes example. 

(In reply to Jan Honza Odvarko [:Honza] from comment #13)
> 
> If I understand correctly, Patrick's suggestion is about merging those two
> parts. I don't have a strong preference, I like both, except that split the
> logic into two parts might be simpler to implement (especially on top of the
> current Menu API we have available).

Agreed, merging the two would require to implement a new Menu widget, maybe not ideal to start.
(In reply to Jan Honza Odvarko [:Honza] PTO 07/23 - 08/08 from comment #13)
> Created attachment 8772378 [details]
> menu.png
> 
> (In reply to Julian Descottes [:jdescottes] from comment #11)
> > (In reply to Jan Honza Odvarko [:Honza] from comment #10)
> > Sounds good except for one point :
> > 
> > > * The user can check/uncheck any item to show/hide any tab.
> Yes, you are right.
> 
> (In reply to Patrick Brosset <:pbro> from comment #12)
> > (In reply to Julian Descottes [:jdescottes] from comment #11)
> > > In my opinion, closing/hiding a tab has to remain separate from the menu.
> > Agreed. There might be a way to merge them though: at the bottom of the
> > menu, have an "edit" item that, once clicked, puts checkboxes next to each
> I like that.
> 
> I have also talked to Joe and he pointed out similar UI/UX in iTunes.
> 
> See the attached screenshot
> - the image on the left shows a context menu with list of tabs
> - it has 'Edit Menu...' item at the bottom
> - when the user picks the 'Edit Menu...' there is additional popup allowing
> the user to pick tabs.
> 
> So, the UI is split into two parts. I. part allowing to select tabs and the
> II. part allowing to show/hide tabs.
> 
> If I understand correctly, Patrick's suggestion is about merging those two
> parts. I don't have a strong preference, I like both, except that split the
> logic into two parts might be simpler to implement (especially on top of the
> current Menu API we have available).
> 
> Honza

The iTunes paradigm has been the one that we've been operating off of for overflow, and what we'd like to move towards for these sorts of issues.

I agree with everyone above me: sounds like a good way to handle this issue, although as Julian says, I don't know what that means for the implementation.
Flags: needinfo?(hholmes)
Product: Firefox → DevTools
I think this is obsolete by now and can be closed. @pbro WDYT?
Flags: needinfo?(pbrosset)
Yes, good point. Let me close it now.
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(pbrosset)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.