Closed Bug 901169 Opened 6 years ago Closed 6 years ago

grey out black boxed source labels

Categories

(DevTools :: Debugger, defect)

25 Branch
x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: fitzgen, Assigned: fitzgen)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

This would really highlight that they are black boxed.
Assignee: nobody → nfitzgerald
Attached patch v1 (obsolete) — Splinter Review
Attachment #785310 - Flags: review?(dcamp)
Why not use black text. To give the feeling of "BLACK"boxing.
Comment on attachment 785310 [details] [diff] [review]
v1

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

I don't really feel qualified to review this, handing to paul.  In particular, I think that GrayText might change per-platform and not necessarily look great with our theming?
Attachment #785310 - Flags: review?(dcamp) → review?(paul)
How about we keep the same color for the text and instead change the background color/gradient of the source in the source list to a dark/black color, thus signifying "blackbox" ?
(In reply to Dave Camp (:dcamp) from comment #3)
> Comment on attachment 785310 [details] [diff] [review]
> v1
> 
> Review of attachment 785310 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't really feel qualified to review this, handing to paul.  In
> particular, I think that GrayText might change per-platform and not
> necessarily look great with our theming?

FWIW, we use GrayText in the breadcrumbs.
I think we should both change the background and gray out or italicize the text.
Comment on attachment 785310 [details] [diff] [review]
v1

Like Dave said, GrayText is platform and theme dependent, when the background color is not. So it's better to hard code the value here.
Attachment #785310 - Flags: review?(paul)
Attached patch v2Splinter Review
* No more GrayText, we use #666 (so metal)

* Italic text

* Not sold on the background changes
Attachment #785310 - Attachment is obsolete: true
Attachment #786397 - Flags: review?(vporof)
Comment on attachment 786397 [details] [diff] [review]
v2

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

I'm not sure I like my "let's italicize the text" idea anymore. Looks cheap. How about simply going for "color: #888;" and be done with it? It looks good to me (using #666 makes it very hard to read imho) r+ with this change if you agree.
Attachment #786397 - Flags: review?(vporof) → review+
https://hg.mozilla.org/mozilla-central/rev/6ac17afa560c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 26
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.