Change menu items divider color to #D7D9DB

RESOLVED FIXED in Firefox 41

Status

()

Firefox for Android
Theme and Visual Design
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: antlam, Assigned: Alexander Ploner, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 41
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8584736 [details]
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.
Blocks: 1127517
Mentor: michael.l.comella@gmail.com
(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)
(Reporter)

Comment 2

3 years ago
(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)
(Assignee)

Comment 3

3 years ago
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)
(Assignee)

Comment 4

3 years ago
Created attachment 8603825 [details] [diff] [review]
bug-1148549-v1.diff
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.
(Assignee)

Comment 8

3 years ago
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
https://hg.mozilla.org/integration/fx-team/rev/199a0e1e1063
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
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
Last Resolved: 3 years ago
status-firefox41: --- → fixed
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.