Closed Bug 1148549 Opened 5 years ago Closed 4 years ago

Change menu items divider color to #D7D9DB

Categories

(Firefox for Android :: Theme and Visual Design, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: antlam, Assigned: alex, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached image 2015-03-27 19.11.15.png
Breaking up the comments in the meta into their own bugs, could make good mentor bugs?

Dividers in between each row item in our overflow menu seem to be #C9D3DC, we should update this to be consistent to our other dividers - #D7D9DB.
(In reply to Anthony Lam (:antlam) from comment #0)
> we should update this to be consistent to our other dividers - #D7D9DB.

Anthony, this doesn't appear to be in the palette - should it be?

It's called "divider_light" in our code currently (but that doesn't mean it can't be changed).
Flags: needinfo?(alam)
(In reply to Michael Comella (:mcomella) from comment #1)
> (In reply to Anthony Lam (:antlam) from comment #0)
> > we should update this to be consistent to our other dividers - #D7D9DB.
> 
> Anthony, this doesn't appear to be in the palette - should it be?
> 
> It's called "divider_light" in our code currently (but that doesn't mean it
> can't be changed).

Sure, we can add it. "Toolbar divider grey"?
Flags: needinfo?(alam)
Hi there!
After many years I definitely want to go a step further in contributing to Mozilla projects, so I took this little bug and fixed it. Setting up the development environment took some time, but it was not too complicated because of the good introductions in the Mozilla Wiki. The more difficult part was (and will be) submitting patches with Mercurial.

I committed my changes locally:

> $ hg parent
> changeset:   243176:ef18ed877f93
> tag:         tip
> user:        Alexander Ploner <ale****atnet.de>
> date:        Sun May 10 14:01:20 2015 +0200
> summary:     Bug 1148549 - Change menu items divider color to #D7D9DB. r=mcomella

Now I need some help to go on. What steps are next?
Should I push the changes to any place of the repository or create a patch file?
Flags: needinfo?(michael.l.comella)
Attachment #8603825 - Flags: review?(michael.l.comella)
Comment on attachment 8603825 [details] [diff] [review]
bug-1148549-v1.diff

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

Hey, Alex - thanks for the patch!

(In reply to Alexander Ploner from comment #3)
> Now I need some help to go on. What steps are next?
> Should I push the changes to any place of the repository or create a patch
> file?

Looks like you did the right thing! Nice!

I made a push to our try test servers (see above).

Once it goes green, feel free to add the checkin-needed keyword [1]. Let me know if you need help reading the results. Note that all patches that use "checkin-needed" must also have an associated green try run.

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Attachment #8603825 - Flags: review?(michael.l.comella) → review+
Assignee: nobody → alex
Flags: needinfo?(michael.l.comella)
(In reply to Anthony Lam (:antlam) from comment #2)
> > Anthony, this doesn't appear to be in the palette - should it be?
> > 
> > It's called "divider_light" in our code currently (but that doesn't mean it
> > can't be changed).
> 
> Sure, we can add it. "Toolbar divider grey"?

I filed bug 1164307 as a follow-up for another mentee.
Thank you for doing the first review and pushing it to the try servers.
Everything turned green, so my first little patch is ready for check in! \o/
Keywords: checkin-needed
Hardware: x86 → All
Alex, do you have a tablet? If so, maybe you'd be interested in working bug 1127578? Otherwise maybe bug 1138560?
https://hg.mozilla.org/mozilla-central/rev/199a0e1e1063
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.