All users were logged out of Bugzilla on October 13th, 2018

[chameleon] InContent Prefs - Update menulist style

RESOLVED FIXED in Firefox 36

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ntim, Assigned: ntim)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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

Comment 1

4 years ago
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
Created attachment 8519469 [details] [diff] [review]
Patch

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.
Created attachment 8520940 [details] [diff] [review]
Patch v2

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
Last Resolved: 4 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.