Closed
Bug 1363753
Opened 8 years ago
Closed 8 years ago
Ensure that the width of panel views do not exceed that of the main view
Categories
(Firefox :: Toolbars and Customization, defect, P1)
Firefox
Toolbars and Customization
Tracking
()
Tracking | Status | |
---|---|---|
firefox55 | --- | verified |
People
(Reporter: mikedeboer, Assigned: mikedeboer)
References
(Blocks 1 open bug)
Details
(Whiteboard: [photon-structure])
Attachments
(1 file)
This should be implemented using a property/ attribute on the photonpanelmultiview that will enable this behavior.
To control the width of the panel main view, we can simply set the min-width and max-width CSS property for each panel individually.
Once the panel is opened for the first time, the main view is shown. From that point forward, it will never change its width for the subviews, and buttons that have labels that are too long will overflow using ellipsis.
Flags: qe-verify+
Comment 1•8 years ago
|
||
Neil's patch in bug 1293242 allows you to use the main view to set the width of the subviews using stack layout.
Depends on: 1293242
Updated•8 years ago
|
Priority: P1 → P2
QA Contact: gwimberly
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8866864 [details]
Bug 1363753 - Add an option for panelmultiviews to keep the width of the mainView synchronized across all other subviews.
https://reviewboard.mozilla.org/r/138452/#review141702
Nice! r=me without the attribute - see below.
::: browser/components/customizableui/PanelMultiView.jsm:390
(Diff revision 1)
> let reverse = !!aPreviousView;
> let previousViewNode = aPreviousView || this._currentSubView;
> let playTransition = (!!previousViewNode && previousViewNode != viewNode);
>
> let dwu, previousRect;
> - if (playTransition) {
> + if (playTransition || this._syncMainViewWidth) {
We can use this.panelViews instead of introducing a separate bool attribute, I think. That should make this a reasonably straightforward patch, I hope? :-)
Attachment #8866864 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request) |
Comment 5•8 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
hg error in cmd: hg rebase -s 624a2ab5b30d -d 080fe566625d: rebasing 395227:624a2ab5b30d "Bug 1363753 - Add an option for panelmultiviews to keep the width of the mainView synchronized across all other subviews. r=Gijs" (tip)
merging browser/components/customizableui/PanelMultiView.jsm
merging browser/themes/shared/customizableui/panelUI.inc.css
warning: conflicts while merging browser/themes/shared/customizableui/panelUI.inc.css! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Updated•8 years ago
|
Iteration: --- → 55.5 - May 15
Comment hidden (mozreview-request) |
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c8f7b07ffdc1
Add an option for panelmultiviews to keep the width of the mainView synchronized across all other subviews. r=Gijs
Comment 8•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Any suggested STR to be able to verify this?
Flags: needinfo?(gijskruitbosch+bugs)
Comment 10•8 years ago
|
||
(In reply to Grover Wimberly IV [:Grover-QA] from comment #9)
> Any suggested STR to be able to verify this?
Yes! If you look at the animations recorded in bug 1364672, you can see that at the end of the "more" and "character encoding" views sliding in, the header/title at the top "jumps" to the right and then back. You can also see that the width of the panel changes after each view is displayed.
In builds with this fixed, neither of those things happen.
Flags: needinfo?(gijskruitbosch+bugs)
Comment 11•8 years ago
|
||
Thanks, Gijs.
Verified on Windows, Mac, and Ubuntu.
Status: RESOLVED → VERIFIED
Updated•8 years ago
|
Flags: qe-verify+
Comment 12•7 years ago
|
||
Is there a bug on file for this being... silly in the Library button on the toolbar? (i.e. jokes apart, there should be a minimum width on the main view or something).
I'd be surprised if this wasn't a more recent regression, caused by something else than this bug, but I couldn't find anything on file and I've also looked at meta-bug 1352110.
Flags: needinfo?(gijskruitbosch+bugs)
Comment hidden (obsolete) |
Comment 14•7 years ago
|
||
(In reply to :Gijs from comment #13)
> (In reply to :Paolo Amadini from comment #12)
> > Is there a bug on file for this being... silly in the Library button on the
> > toolbar? (i.e. jokes apart, there should be a minimum width on the main view
> > or something).
> >
> > I'd be surprised if this wasn't a more recent regression, caused by
> > something else than this bug, but I couldn't find anything on file and I've
> > also looked at meta-bug 1352110.
>
> bug 1370082 addresses this.
Err, I meant bug 1370580.
You need to log in
before you can comment on or make changes to this bug.
Description
•