Bug 187657 – Web Inspector: Dark Mode: bezier curve editor should be updated

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Created attachment 344975 [details]
[Screenshot] Bug

The curve line should be white.
The blue lines should be lighter.
The lines that are currently light should be darker.

[Radar WebKit Bug Importer]

<rdar://problem/42179060>

[Jamal Nasser]

Created attachment 349707 [details]
Patch

[Jamal Nasser]

Created attachment 349708 [details]
Screenshot with patch

[Nikita Vasilyev]

Comment on attachment 349707 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349707&action=review

Thank you for working on this. Just a few small corrections below.

> Source/WebInspectorUI/ChangeLog:3
> +        Added CSS rules to DarkMode.css in order to override the normal ones in BezierEditor.css

This line should match the title of the Bugzilla bug:
Web Inspector: Dark Mode: bezier curve editor should be updated

If you'd like to add a description, it should go after "Reviewed by" line. See other changelog items for examples.

> Source/WebInspectorUI/UserInterface/Views/DarkMode.css:1171
> +        border-top: 4px solid white;

border-top-color: white;

> Source/WebInspectorUI/UserInterface/Views/DarkMode.css:1183
> +        border-bottom: 1px solid var(--text-color-tertiary);;

border-bottom-color: var(--text-color-tertiary);

[Jamal Nasser]

Created attachment 349709 [details]
Patch

[Jamal Nasser]

Thanks for the fast review and for being patient with me!

[Nikita Vasilyev]

Comment on attachment 349709 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349709&action=review

Looks good besides these two comments below. I'm not a reviewer, so I can't r+.

> Source/WebInspectorUI/UserInterface/Views/DarkMode.css:1171
> +        border-top-color: white;

In the light mode, this is a light gray border. In the dark mode, this should be dark gray. I suggest to use `var(--text-color-tertiary)`.

> Source/WebInspectorUI/UserInterface/Views/DarkMode.css:1183
> +        border-bottom-color: var(--text-color-tertiary);;

Two semicolons at the end of the line. One is sufficient.

[Matt Baker]

Once Nikita is satisfied, I'll r+.

[Jamal Nasser]

Created attachment 349711 [details]
Patch

[Jamal Nasser]

Fixed, I was going to ask about the color; glad you mentioned that :)

[Nikita Vasilyev]

Comment on attachment 349711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349711&action=review

Looks good, just one nitpick below.

> Source/WebInspectorUI/ChangeLog:8
> +        Added CSS rules to DarkMode.css in order to override the normal ones in BezierEditor.css

I think this is fine without any description. I can see some CSS rules were added to DarkMode.css a couple of lines below, no need to mention that.

If you really want to add a description, explain what was the problem (e.g. "the bezier curve was hard to see because it was black on dark grey background") and how you solved it.

[Jamal Nasser]

Created attachment 349712 [details]
Patch

[Nikita Vasilyev]

Looks good! Thank you for doing this!

(In reply to Matt Baker from comment #8)
> Once Nikita is satisfied, I'll r+.

Looks r+-worthy to me!

[Matt Baker]

Comment on attachment 349712 [details]
Patch

rs=me

[Jamal Nasser]

(In reply to Nikita Vasilyev from comment #13)
> Looks good! Thank you for doing this!
> 
> (In reply to Matt Baker from comment #8)
> > Once Nikita is satisfied, I'll r+.
> 
> Looks r+-worthy to me!

Thanks for guiding me through this

[WebKit Commit Bot]

Comment on attachment 349712 [details]
Patch

Clearing flags on attachment: 349712

Committed r235998: <https://trac.webkit.org/changeset/235998>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Nikita Vasilyev]

Comment on attachment 349712 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349712&action=review

> Source/WebInspectorUI/UserInterface/Views/DarkMode.css:1193
> +    

Next time, please configure your code editor to NOT leave white space on empty lines.