Closed Bug 1018796 Opened 8 years ago Closed 7 years ago

[chameleon] InContent Prefs - Update menulist style

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 36

People

(Reporter: ntim, Assigned: ntim)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

To match project chameleon, we should :
- Get rid of the background gradient (we're flattening the UI)
- Update the dropdown arrow glyph
- Update the dropdown list style
Blocks: 1014208
Assignee: nobody → ntim007
Assignee: ntim007 → nobody
Summary: [chameleon] InContent Prefs - Get rid of background gradient and update menulist style → [chameleon] InContent Prefs - Update menulist style
I was going to fill a separate bug, but then I came across this one so I'll report it here.

Menuitems (40px) don't use the same height as menulist (30px). This should be corrected (decrease the height of menuitems to 30px). Screenshot at http://imgur.com/7SqqG7b
Attached patch Patch (obsolete) — Splinter Review
Changes :
- menulist arrow (to match new specs)
- Fixes high contrast color for menulist arrow
- Fixes issue in Comment 1
- Uses flat colors (colors from the Project Chameleon palette), instead of gradients (the gradients are leftovers from the older chameleon UI)
- Removes the text-shadows from the older chameleon UI (leftovers as well)
Assignee: nobody → ntim007
Attachment #8519469 - Flags: review?(jaws)
Status: NEW → ASSIGNED
Comment on attachment 8519469 [details] [diff] [review]
Patch

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

This is super close to r+, just make the following change, test it, and put it up for review and I'll take another look. Nice job.

::: toolkit/themes/shared/in-content/dropdown.svg
@@ +12,5 @@
> +      fill: GrayText;
> +    }
> +  </style>
> +  <path fill-rule="evenodd" clip-rule="evenodd" d="M12,6l-4.016,4L4,6H12z" id="dropdown"/>
> +  <path fill-rule="evenodd" clip-rule="evenodd" d="M12,6l-4.016,4L4,6H12z" class="disabled" id="dropdown-disabled"/>

We don't need to duplicate the paths here (see how d="..." is defined twice above, both with the same path).

To share the path, we can do the following:

<style>
use {
  fill: menutext;
}
use[id$="-active"] {
  fill: -moz-menuhovertext;
}
use[id$="-disabled"] {
  fill: graytext;
}
</style>
<defs style="display:none;">
  <path id="dropdown-shape" fill-rule="evenodd" clip-rule="evenodd" d="M12,6l-4.016,4L4,6H12z"/>
</defs>
<use id="dropdown" xlink:href="dropdown-shape"/>
<use id="dropdown-disabled" xlink:href="dropdown-shape"/>

You can see /browser/themes/linux/content-contextmenu.svg for an example of this already in use.
Attachment #8519469 - Flags: review?(jaws) → feedback+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3)

Edit, you shouldn't need the `use[id$="-active"]` rule above.
Attached patch Patch v2Splinter Review
Addressed review comment.
Attachment #8519469 - Attachment is obsolete: true
Attachment #8520940 - Flags: review?(jaws)
Attachment #8520940 - Flags: review?(jaws) → review+
Whiteboard: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/d53d4abae160
Whiteboard: checkin-needed → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d53d4abae160
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
Duplicate of this bug: 1052318
You need to log in before you can comment on or make changes to this bug.