Closed Bug 944731 Opened 10 years ago Closed 10 years ago

Add a border to color swatches

Categories

(DevTools :: Inspector, enhancement)

enhancement
Not set
normal

Tracking

(firefox32 verified)

VERIFIED FIXED
Firefox 32
Tracking Status
firefox32 --- verified

People

(Reporter: sjw+bugzilla, Assigned: johannes)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good-first-bug][lang=css][mentor=mratcliffe])

Attachments

(3 files)

Attached image current state
The color swatches should have a border for a better contrast. On dark theme it should be light, on light theme it should be dark.
It might be perfect, if the border only appears if its needed.
Blocks: 918716
Blocks: 711947
Could you keep an eye on this? I think its very easy to fix and would improve the handling a lot.
Flags: needinfo?(mratcliffe)
There is already a border (actually an outline) but it is maybe a little too subtle.

For whoever fixes this:
- Search for ".ruleview-colorswatch,"
- The two CSS rules that you find have something like "box-shadow: 0 0 0 1px #EFEFEF;"... one for the light theme and one for the dark theme.
- Change the rgb value to find something that fits in each theme.
Flags: needinfo?(mratcliffe)
Whiteboard: [good-first-bug][lang=css][mentor=mratcliffe]
This should fix it. Could you please assign me to the bug and check it in?
Attachment #8414695 - Flags: review?(mratcliffe)
Not sure what a 1px border looks like on a high resolution display.
It looks nice on my 17" notebook. Should we change it to a relative width? It was already 1px before my change by the way.
Assignee: nobody → johannes
Comment on attachment 8414695 [details] [diff] [review]
colorborders.patch

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

(In reply to sjw from comment #4)
> Not sure what a 1px border looks like on a high resolution display.

It looks nice on both high and low resolution displays, great job Johannes!
Attachment #8414695 - Flags: review?(mratcliffe) → review+
Whiteboard: [good-first-bug][lang=css][mentor=mratcliffe] → [good-first-bug][lang=css][mentor=mratcliffe][land-in-fx-team]
Whiteboard: [good-first-bug][lang=css][mentor=mratcliffe][land-in-fx-team] → [good-first-bug][lang=css][mentor=mratcliffe]
Status: NEW → ASSIGNED
Keywords: checkin-needed
Johannes, another good color swatch related bug if you are interested is Bug 956044 - adding a checkered background to show up behind colors with some transparency.
https://hg.mozilla.org/integration/fx-team/rev/7d5b7d673c87
Keywords: checkin-needed
Whiteboard: [good-first-bug][lang=css][mentor=mratcliffe] → [good-first-bug][lang=css][mentor=mratcliffe][fixed-in-fx-team]
I already had a look at it, but it is quiet difficult as a "background-image" is always behind a "background-color" and I have no idea how to fix this.
(In reply to Johannes Mittendorfer from comment #9)
> I already had a look at it, but it is quiet difficult as a
> "background-image" is always behind a "background-color" and I have no idea
> how to fix this.

It can be accomplished with a second element, where the container has the checker background - like this: http://jsfiddle.net/bgrins/7aWU8/.  It may require some small JS changes to make sure the markup is being set up correctly, but it shouldn't be too bad.
https://hg.mozilla.org/mozilla-central/rev/7d5b7d673c87
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [good-first-bug][lang=css][mentor=mratcliffe][fixed-in-fx-team] → [good-first-bug][lang=css][mentor=mratcliffe]
Target Milestone: --- → Firefox 32
Attached image color-swatch-border.png
Please see the screenshot, I don't think the border looks good… the top left is thicker as the bottom right… Tested with OS X 10.9 HiDPI.
(In reply to Sören Hentzschel from comment #12)
> Created attachment 8416918 [details]
> color-swatch-border.png
> 
> Please see the screenshot, I don't think the border looks good… the top left
> is thicker as the bottom right… Tested with OS X 10.9 HiDPI.

Try changing the size by one pixel... everything should look properly centered then.
Hi,

I've been able to reproduce it in the Nightly build 20131129 on both Debian Linux x68_64 and Windows 7 x86_64. And I can confirm it's fixed in latest Nightly (32.0a1 2014-05-28) in both platforms.

As these are 2 out of 3 platforms, I'm marking this as verified.

Cheers,
Francesca
Status: RESOLVED → VERIFIED
QA Whiteboard: [bugday-20140528]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.