Closed Bug 835518 Opened 7 years ago Closed 7 years ago

Work - Move the Metro Firefox options into a flyout sidebar

Categories

(Firefox for Metro Graveyard :: General, defect)

All
Windows 8.1
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

(Keywords: ux-interruption, Whiteboard: feature=work [completed-elm])

Attachments

(3 files)

These patches move our options list from a full-screen panel to a flyout sidebar panel.  The content of the options panel is unchanged; this bug is just for moving them into the sidebar.

The first patch makes some minor changes to the flyoutpanel binding to make it easier to re-use for more complex content.  It mades the content area scrollable, moves some common styles into the binding, and adds an event so UI code can detect when the flyout is opened.
Attachment #707248 - Flags: review?(netzen)
Note: The XUL content is unchanged; I just moved it to a different container.

I tweaked the styles to work a little better in the new location, and got rid of some unused styles.
Attachment #707250 - Flags: review?(netzen)
I noticed a bug in input.js which affected the options panel on displays of a certain height (which happened to include my X1):

The isDraggable method here was comparing the scrollable height to the bounding height of the "target element" (in this case, the <flyoutpanel>) instead of the height of the anonymous scrollbox.  For existing uses of this code the two dimensions were the same, but for the flyoutpanel the heights are different.
http://hg.mozilla.org/projects/elm/file/78285a7a3cab/browser/metro/base/content/input.js#l466

By exposing the scrollbox itself, we can make the correct comparison here.  This also lets us simplify the code for some other bindings that contain anonymous scrollboxes.
Attachment #707256 - Flags: review?(netzen)
Comment on attachment 707248 [details] [diff] [review]
part 1: changes to flyoutpanel binding

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

::: browser/metro/theme/flyoutpanel.css
@@ +24,5 @@
>    visibility: visible;
>  }
>  
> +/* XUL flexbox layout doesn't work in a position:fixed container, so we have
> + * this normally-positioned inside for layout purposes. */

ah! thanks for explaining this. I fought with this for a while and didn't understand why flex wasn't working.
Attachment #707248 - Flags: review?(netzen) → review+
Attachment #707250 - Flags: review?(netzen) → review+
Comment on attachment 707256 [details] [diff] [review]
part 3: Fix a bug with anonymous scrollboxes

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

I'm not 100% confident with reviewing this patch, but since it's going on elm I'll go with it.  Feel free to get a second review on this one if you think it's needed.
Attachment #707256 - Flags: review?(netzen) → review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [feature=work][completed-elm] → feature=work [completed-elm]
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.