| Summary: | Web Inspector: Dark Mode: bezier curve editor should be updated | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||||||||||
| Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | commit-queue, inspector-bugzilla-changes, jamaln, mattbaker, webkit-bug-importer | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | All | ||||||||||||||||
| OS: | All | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
Created attachment 349707 [details]
Patch
Created attachment 349708 [details]
Screenshot with patch
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); Created attachment 349709 [details]
Patch
Thanks for the fast review and for being patient with me! 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. Once Nikita is satisfied, I'll r+. Created attachment 349711 [details]
Patch
Fixed, I was going to ask about the color; glad you mentioned that :) 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. Created attachment 349712 [details]
Patch
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! Comment on attachment 349712 [details]
Patch
rs=me
(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 Comment on attachment 349712 [details] Patch Clearing flags on attachment: 349712 Committed r235998: <https://trac.webkit.org/changeset/235998> All reviewed patches have been landed. Closing bug. 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. |
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.