861088 - Make the subview widgets show their contents as an arrow panel when the widget is not located in the Menu Panel

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

Currently, the new History widget doesn't do anything in the nav-bar. One option is to have it open the History side-bar. The other option is to open the Library at the History view.

There are probably other options. We should decide on one and implement it.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Or show the sub-view in a panel - consistency FTW (and it'd be more contextual than opening a side-bar or the Library). I'd been thinking we should eventually abstract out the sub-views somehow, so this kind of thing is possible.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: WIP patch

The styling of the panel isn't right, but I think this gets us in the right direction.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 739150 [details] [diff] [review]
WIP patch

Review of attachment 739150 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, that'd work nicely.

For robustness, might want to add a sanity check for the panel, when opening a subview - ensuring that the node is where we expect it to be (and if it isn't there, move it).

::: browser/base/content/panelUI.js
@@ +221,5 @@
> +        characterData: true,
> +        childList: true,
> +        subtree: true
> +      });
> +    } else if (aPlacementArea == CustomizableUI.AREA_NAVBAR) {

This shouldn't be checking it's in the navbar, but rather that it's in an area that isn't the panel.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

This fixes the nits that Blair brought up in the feedback pass and gets the content of the panel to match the styling of the subview.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Also tested it with the Bookmarks widget and it works well.

Comment 6

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

Comment on attachment 743296 [details] [diff] [review]
Patch

Review of attachment 743296 [details] [diff] [review]:
-----------------------------------------------------------------

Looking forward to making use of this in bug 855805. :) Thanks.

::: browser/components/customizableui/content/panelUI.js
@@ +174,5 @@
>  
>    /**
>     * Shows a subview in the panel with a given ID.
>     *
>     * @param aViewId the ID of the subview to show.

The function documentation should be updated.

::: browser/themes/shared/customizableui/panelUIOverlay.inc.css
@@ +187,5 @@
>    display: none;
>  }
>  
> +#customizationui-widget-panel {
> +  max-width: 23em;

If this is supposed to match the menu panel, then this should be @menuPanelWidth@, as opposed to 23em.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/projects/jamun/rev/ed00f90d5e76

Comment 8

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

https://hg.mozilla.org/projects/ux/rev/ed00f90d5e76

Comment 9

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/ed00f90d5e76