Closed Bug 922003 Opened 11 years ago Closed 11 years ago

Move panelMenu gutter width to a constant next to menuPanelWidth

Categories

(Firefox :: Toolbars and Customization, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: mikedeboer, Assigned: i.am.cust0dian)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug][mentor=mikedeboer][lang=css][Australis:M9][Australis:P-])

Attachments

(1 file, 1 obsolete file)

Since bug 867585 was implemented, the width of the MenuPanel is defined as a constant in panelUIOverlay.css.
In bug 872548 we noticed that another constant width of 38px was used for the gutter that the user can click to go back to the main view if they're viewing a subview.
What need to happen here is move that 38px into a constant next to menuPanelWidth in panelUIOverlay.css.

To work on this bug, you'll need to clone the HG repo located at https://hg.mozilla.org/projects/ux/. The files in question are located at /browser/themes/linux/customizableui/PanelUIOverlay.css, /browser/themes/osx/customizableui/PanelUIOverlay.css and /browser/themes/windows/customizableui/PanelUIOverlay.css
Attached patch Bug-922003: Patch 1 (obsolete) — Splinter Review
Hello, I'd like to help but not entirely sure how to proceed.
Attachment #814548 - Flags: feedback?(mdeboer)
Hi Serg! Thanks for taking this bug, your help is VERY appreciated!

So first thing Jared did is set the appropriate flag(s) on the patch you submitted, in this case asking me for feedback.
The second thing I did was assigning the bug to you and setting the status to ASSIGNED.
(I'm explaining this, because it'll help you with your next bug, which I hope you will take on after success here!)

Now I will download and try the patch you wrote and provide you with the information needed to make it final and able to land on the UX branch.

Stay tuned!
Assignee: nobody → i.am.cust0dian
Status: NEW → ASSIGNED
Whiteboard: [good first bug][mentor=mikedeboer][lang=css][Australis:M?][Australis:P?] → [good first bug][mentor=mikedeboer][lang=css][Australis:M?][Australis:P-]
Comment on attachment 814548 [details] [diff] [review]
Bug-922003: Patch 1

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

::: browser/themes/linux/customizableui/panelUIOverlay.css
@@ +4,4 @@
>  
>  %filter substitution
>  %define menuPanelWidth 21em
> +%define exitSubviewGutterWidth 38px

Please move all three lines above into `browser/themes/shared/customizableui/panelUIOverlay.inc.css` and remove them from the windows & OSX theme files as well.

(You can paste these lines above `%include ../browser.inc` in that file.)
Attachment #814548 - Flags: feedback?(mdeboer) → feedback+
Move repeating lines from system-specific files into include file.
Attachment #814548 - Attachment is obsolete: true
Attachment #815589 - Flags: review?(mdeboer)
Comment on attachment 815589 [details] [diff] [review]
Bug-922003: Patch 2

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

That looks great! Thanks for working on this :)

One small nit: When I check this in for you, I will change the commit message to the following:
`Bug 922003: Move width of the gutter, used to exit subview, to a constant. r=mikedeboer`

In other words: prefixed with the bug number and suffixed with review metadata; in this case that I r+'ed this patch.

I hope you're eager to work on (many) more bugs in the near future, now you made this one a success!
Attachment #815589 - Flags: review?(mdeboer) → review+
https://hg.mozilla.org/projects/ux/rev/d379376cef91
Whiteboard: [good first bug][mentor=mikedeboer][lang=css][Australis:M?][Australis:P-] → [good first bug][mentor=mikedeboer][lang=css][Australis:M?][Australis:P-][fixed-in-ux]
Thank you, Mike!

One question: is there a list of people who I can ask for review and check-ins or I'll just know them as I go (can't seem to find it in the wiki)?

And is it my responsibility to mark bug as resolved now or do I wait for something else?
(In reply to Serg Nesterov from comment #7)
> Thank you, Mike!
> 
> One question: is there a list of people who I can ask for review and
> check-ins or I'll just know them as I go (can't seem to find it in the wiki)?

Yes, there's a list. You can follow this screencast to see how you access the list: http://screencast.com/t/z0DPDlNM6n9
 
> And is it my responsibility to mark bug as resolved now or do I wait for
> something else?

For Australis bugs, we are waiting to mark them as resolved until they are merged to mozilla-central. In general though, you won't need to mark bugs as resolved. A tree sheriff will mark them as resolved when they perform the merge.
(In reply to Jared Wein [:jaws] from comment #8)
> (In reply to Serg Nesterov from comment #7)
> > Thank you, Mike!
> > 
> > One question: is there a list of people who I can ask for review and
> > check-ins or I'll just know them as I go (can't seem to find it in the wiki)?
> 
> Yes, there's a list. You can follow this screencast to see how you access
> the list: http://screencast.com/t/z0DPDlNM6n9
>  
> > And is it my responsibility to mark bug as resolved now or do I wait for
> > something else?
> 
> For Australis bugs, we are waiting to mark them as resolved until they are
> merged to mozilla-central. In general though, you won't need to mark bugs as
> resolved. A tree sheriff will mark them as resolved when they perform the
> merge.

I see, thank you, Jared!
https://hg.mozilla.org/mozilla-central/rev/d379376cef91
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=mikedeboer][lang=css][Australis:M?][Australis:P-][fixed-in-ux] → [good first bug][mentor=mikedeboer][lang=css][Australis:M9][Australis:P-]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.