Last Comment Bug 765221 - White text shadow for lightweight themes with dark text looks weird
: White text shadow for lightweight themes with dark text looks weird
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Themes (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: mozilla16
Assigned To: Dão Gottwald [:dao]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-15 06:34 PDT by Dão Gottwald [:dao]
Modified: 2012-07-13 05:32 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
patch (1.77 KB, patch)
2012-06-15 06:34 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
patch v2 (1.76 KB, patch)
2012-07-12 11:23 PDT, Dão Gottwald [:dao]
shorlander: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2012-06-15 06:34:53 PDT
Created attachment 633496 [details] [diff] [review]
patch

A shadow can't be white and the text doesn't look like it's inset either... It makes more sense for the white "shadow" to be above the text as a highlight.
Comment 1 Stephen Horlander [:shorlander] 2012-07-12 11:16:57 PDT
Is there a specific reason for the horizontal offset? Using just a vertical offset would be more consistent with our other highlight/shadow effects and seems to have about the same effect on legibility.
Comment 2 Dão Gottwald [:dao] 2012-07-12 11:23:35 PDT
Created attachment 641543 [details] [diff] [review]
patch v2

no specific reason
Comment 3 Stephen Horlander [:shorlander] 2012-07-12 11:37:33 PDT
Comment on attachment 641543 [details] [diff] [review]
patch v2

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

Looks good, thank you!
Comment 5 Dão Gottwald [:dao] 2012-07-12 12:08:13 PDT
Comment on attachment 641543 [details] [diff] [review]
patch v2

[Approval Request Comment]
Not a regression fix but risk-free UI polish. Just changing the vertical and horizontal offset parameters of the text shadow.
Comment 6 Alex Keybl [:akeybl] 2012-07-12 14:43:42 PDT
Comment on attachment 641543 [details] [diff] [review]
patch v2

[Triage Comment]
No expected fallout from this polish and still early in the cycle, approved for Aurora 15.
Comment 8 Ed Morley [:emorley] 2012-07-13 05:32:37 PDT
https://hg.mozilla.org/mozilla-central/rev/ab6a89ec10fe

Note You need to log in before you can comment on or make changes to this bug.