Closed
Bug 992637
Opened 10 years ago
Closed 10 years ago
Remove the background color from subview headers and make them all-caps instead such that the background matches the panel arrow color
Categories
(Firefox :: Theme, defect)
Tracking
()
People
(Reporter: micmon, Assigned: dao)
References
(Blocks 1 open bug)
Details
(Whiteboard: [Australis:P-])
Attachments
(3 files, 1 obsolete file)
12.23 KB,
image/png
|
Details | |
802 bytes,
patch
|
jaws
:
review+
dao
:
checkin+
|
Details | Diff | Splinter Review |
717 bytes,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
Open the help or developer menu for example. The new menu will slide in from the right side and have a darker header than before. The arrow above the whole menu is still using the old (lighter) color).
Assignee | ||
Updated•10 years ago
|
Whiteboard: dupeme
Assignee | ||
Updated•10 years ago
|
Blocks: australis-cust
Comment 1•10 years ago
|
||
The arrow is an image, and so can't really adapt to varying panel colors. We'd need to use <canvas> or SVG and dynamically update it. (This has been an issue for Social API panels and other places, where the panel content is essentially an <iframe> and can have anything in it.)
Updated•10 years ago
|
Whiteboard: dupeme → dupeme [Australis:P-]
Assignee | ||
Comment 2•10 years ago
|
||
like bug 1057030
Assignee | ||
Updated•10 years ago
|
Iteration: --- → 35.2
Points: --- → 1
Flags: qe-verify+
Flags: firefox-backlog+
OS: Linux → All
Hardware: x86_64 → All
Comment 3•10 years ago
|
||
Comment on attachment 8495482 [details] [diff] [review] menupanel-arrow.diff Review of attachment 8495482 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure if we want to use a gradient here. Much of this design was to use flat colors and we don't have many gradients in the menu. Philipp/Stephen, are you OK with this change? Or should change the color of these headers to match the arrow and thus change the color of the body area to be different?
Attachment #8495482 -
Flags: ui-review?(shorlander)
Attachment #8495482 -
Flags: ui-review?(philipp)
Comment 4•10 years ago
|
||
I had meant to include a screenshot of it for Philipp or Stephen to get their response quicker. Here it is, http://screencast.com/t/dVZNVHhVnw7o
Assignee | ||
Comment 5•10 years ago
|
||
The current color mismatch is obviously unintended and somewhat jarring. Can we agree that the gradient is an improvement over that and get this landed? Changing the subview colors more broadly can be investigated in a different bug.
Comment 6•10 years ago
|
||
It has existed since v29, so I don't see the urgency in fixing it immediately before we get a better design. Ideally we would be able to get the color of the arrow to match the grey instead of fading everything in to white.
Assignee | ||
Comment 7•10 years ago
|
||
I didn't say it's urgent, it's just a bug that we should fix while not ruling out that there might be a better fix down the road. That this bug existed for so long is a little bit embarrassing and not a good reason to wait even longer. (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > Ideally we would be able to get > the color of the arrow to match the grey instead of fading everything in to > white. I disagree that we should start shipping custom arrow images on a per-panel basis. That would be a pretty significant maintenance burden that can be avoided easily when designing stuff, by taking into account that we have a standard panel background that's expected to connect with the arrow. In this particular case changing arrow would be even more dubious since the main view and the sub view aren't consistent with what backgrounds the put next to the arrow, so I guess the arrow image would need to be swapped dynamically.
Comment 8•10 years ago
|
||
If we switched to using an SVG arrow then the color of the arrow could be changed dynamically.
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8) > If we switched to using an SVG arrow then the color of the arrow could be > changed dynamically. How so?
Comment 10•10 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #9) > (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8) > > If we switched to using an SVG arrow then the color of the arrow could be > > changed dynamically. > > How so? Similar to how we did http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/content-contextmenu.svg?raw=1 with http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/contextmenu.inc.css
Assignee | ||
Comment 11•10 years ago
|
||
We'd still need to fork it to browser. Anyway, that's significantly more work than what my patch does and since (like you argued) this bug isn't a priority, I don't see us spending time on this anytime soon, and therefore I'd like to land my stop-gap solution.
Updated•10 years ago
|
Iteration: 35.2 → 35.3
Comment 12•10 years ago
|
||
Comment on attachment 8495482 [details] [diff] [review] menupanel-arrow.diff > <MattN> jaws: Is there a way to change the doorhanger arrow color? i.e. how is that color specified? > <MattN> ah, nvm, I see it's an image > <MattN> I somehow overlooked .panel-arrowbox > <jaws> i would prefer that we went the SVG route, since we are now using SVG in other places of the browser > <shorlander> jaws: I agree with you. It's not pressing and I would rather wait for the right design than do that gradient > <shorlander> I had some ideas, and I know mmaslaney did some design work around that > <shorlander> We talked about just removing the grey area anyway since it was too similar to the footer button. > <shorlander> jaws, MattN: Are there really any problems going the SVG route? Just switch out the graphic if panel has a header attribute? > <MattN> just not a priority probably > <jaws> shorlander: dao says that the code would need to be forked to browser. i'm not sure why, as other products may want the themeing ability > <MattN> I agree > <MattN> not sure why
Attachment #8495482 -
Flags: ui-review?(shorlander)
Attachment #8495482 -
Flags: ui-review?(philipp)
Attachment #8495482 -
Flags: ui-review-
Attachment #8495482 -
Flags: review?(jaws)
Comment 13•10 years ago
|
||
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12) From http://logs.glob.uno/?c=mozilla%23fx-team&s=1+Oct+2014&e=1+Oct+2014&h=svg#c171128
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8495482 [details] [diff] [review] menupanel-arrow.diff (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12) > > <jaws> shorlander: dao says that the code would need to be forked to browser. i'm not sure why, as other products may want the themeing ability It's not about the ability, it's about the particular color this particular panel wants. If you read the previous comments, you'll see that I wasn't presenting the gradient as the ideal solution. It's the simplest solution and it's better than the color mismatch. I can't justify spending more time on a more complicated solution right now, so we either take this patch, or someone else takes this bug, or it goes back to zombie mode and we keep shipping the obvious color mismatch.
Attachment #8495482 -
Flags: ui-review- → ui-review?(shorlander)
Comment 15•10 years ago
|
||
The other option that shorlander mentioned in comment #12 was that we could look in to removing the header color altogether. "We talked about just removing the grey area anyway since it was too similar to the footer button." Stephen, if we went that route, would we need to make any other color changes to the rest of the panel?
Updated•10 years ago
|
QA Contact: cornel.ionce
Comment 16•10 years ago
|
||
Comment on attachment 8495482 [details] [diff] [review] menupanel-arrow.diff Review of attachment 8495482 [details] [diff] [review]: ----------------------------------------------------------------- I would rather not introduce a gradient here. We did talk about this issue in post Australis follow-up. The solution that came up was to remove the background-color and give the title an all caps treatment. Solves this problem and removes any confusion with the actionable footer. http://cl.ly/image/20341S04420T The other alternative would be to change the color of the arrow based on whether or not a subview is open.
Attachment #8495482 -
Flags: ui-review?(shorlander) → ui-review-
Assignee | ||
Comment 17•10 years ago
|
||
Attachment #8495482 -
Attachment is obsolete: true
Attachment #8503003 -
Flags: review?(jaws)
Comment 18•10 years ago
|
||
Comment on attachment 8503003 [details] [diff] [review] uppercase.diff Review of attachment 8503003 [details] [diff] [review]: ----------------------------------------------------------------- Tested the patch out, looks good.
Attachment #8503003 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/08261d2edbed
Summary: Color of arrow in slide menus → Remove the background color from subview headers and make them all-caps instead such that the background matches the panel arrow color
Whiteboard: dupeme [Australis:P-] → [Australis:P-]
https://hg.mozilla.org/mozilla-central/rev/08261d2edbed
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8503003 [details] [diff] [review] uppercase.diff Approval Request Comment [Feature/regressing bug #]: australis [User impact if declined]: graphical glitch [Describe test coverage new/current, TBPL]: no test coverage [Risks and why]: pretty low, small CSS patch [String/UUID change made/needed]: none
Attachment #8503003 -
Flags: approval-mozilla-aurora?
Comment 22•10 years ago
|
||
pike/flod - Does transforming the menu title to all caps have any impact on l10n?
Flags: needinfo?(l10n)
Flags: needinfo?(francesco.lodolo)
Comment 23•10 years ago
|
||
(In reply to Lawrence Mandel [:lmandel] from comment #22) > pike/flod - Does transforming the menu title to all caps have any impact on > l10n? Yes, it's not safe. See bug 830002 for a nice read.
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 24•10 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #23) > (In reply to Lawrence Mandel [:lmandel] from comment #22) > > pike/flod - Does transforming the menu title to all caps have any impact on > > l10n? > > Yes, it's not safe. See bug 830002 for a nice read. So there was an issue with Greek which should be fixed and there's an issue with Irish where the first letter may be lowercase to express a semantic difference. Seems like we could avoid the latter issue by using font-variant: small-caps instead of text-transform: uppercase?
Comment 25•10 years ago
|
||
I think it works for most locales, but not all of them (exactly like uppercase transformation) https://groups.google.com/forum/#!msg/mozilla.dev.l10n.web/UDjtZgvH3Lg/O6T906KaJTQJ > The translation in the PO file has the word "t-eolas" in lowercase, but > on the button it looks like "T-EOLAS" which is goofy looking - these prefix > letters are never capitalized in Irish; it should look like "tEolas" > or even "tEOLAS" > (note no hyphen in either case with an uppercase "E").
Assignee | ||
Comment 26•10 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #25) > I think it works for most locales, but not all of them (exactly like > uppercase transformation) > https://groups.google.com/forum/#!msg/mozilla.dev.l10n.web/UDjtZgvH3Lg/ > O6T906KaJTQJ > > > The translation in the PO file has the word "t-eolas" in lowercase, but > > on the button it looks like "T-EOLAS" which is goofy looking - these prefix > > letters are never capitalized in Irish; it should look like "tEolas" > > or even "tEOLAS" > > (note no hyphen in either case with an uppercase "E"). I specifically referred to Irish in my previous comment. As I understand it, small-caps should preserve the distinction between the lowercase t and the uppercase E: ᴛEᴏʟᴀs / ᴛEOLAS
Comment 27•10 years ago
|
||
It's an issue if the original word has an hyphen, and the uppercase/small-caps version doesn't need one. Again, we're probably talking about edge cases, we might also want to NI Kevin to shed some light on this.
Assignee | ||
Comment 28•10 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #27) > It's an issue if the original word has an hyphen, and the > uppercase/small-caps version doesn't need one. font-variant: small-caps would transform t-eolas to ᴛ-ᴇᴏʟᴀs, i.e. all caps would be small. Technically the word doesn't become capitalized, it's just a different stylistic representation of the lowercase word.
Assignee | ||
Comment 29•10 years ago
|
||
Attachment #8503560 -
Flags: review?(jaws)
Assignee | ||
Updated•10 years ago
|
Attachment #8503003 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Flags: needinfo?(l10n)
Comment 30•10 years ago
|
||
Reopening as it looks like we need a further change or a backout on m-c.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•10 years ago
|
Attachment #8503003 -
Flags: checkin+
Assignee | ||
Comment 31•10 years ago
|
||
Comment on attachment 8503560 [details] [diff] [review] followup, use small-caps Who wants to review this over the weekend?
Attachment #8503560 -
Flags: review?(mconley)
Attachment #8503560 -
Flags: review?(gijskruitbosch+bugs)
Updated•10 years ago
|
Attachment #8503560 -
Flags: review?(mconley)
Attachment #8503560 -
Flags: review?(jaws)
Attachment #8503560 -
Flags: review?(gijskruitbosch+bugs)
Attachment #8503560 -
Flags: review+
Assignee | ||
Comment 32•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/1ceec2777843
Comment 33•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1ceec2777843
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment 34•10 years ago
|
||
The background color of the subviews header is removed from latest Nightly, build ID: 20141012030203. Tested on Windows 7 64-bit, Mac OS X 10.8.5 and Ubuntu 14.04 32-bit.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•