Bug 234036

Summary: Web Inspector: Increase padding around icons in Alignment editor
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, hi, inspector-bugzilla-changes, pangle, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
[Image] Before
none
[Image] After
none
Patch
none
[Image] After, 1x
none
[Image] RTL, Dark mode none

Description Nikita Vasilyev 2021-12-08 14:36:06 PST
Increase margin and padding around the icons, making the icons more visually appealing and easier to distinguish and from each other.
Comment 1 Radar WebKit Bug Importer 2021-12-15 14:37:17 PST
<rdar://problem/86543279>
Comment 2 Nikita Vasilyev 2022-01-10 13:06:40 PST
Created attachment 448791 [details]
Patch
Comment 3 Nikita Vasilyev 2022-01-10 13:06:59 PST
Created attachment 448792 [details]
[Image] Before
Comment 4 Nikita Vasilyev 2022-01-10 13:07:24 PST
Created attachment 448793 [details]
[Image] After
Comment 5 Devin Rousso 2022-01-10 13:31:44 PST
Comment on attachment 448791 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:3
> +        Web Inspector: Revamp visual design of Alignment editor

Is this really a "revamp"?  Why not something more descriptive like "Web Inspector: increase padding around icons in Alignment editor"?

> Source/WebInspectorUI/UserInterface/Images/AlignContentStretch.svg:4
> +    <rect x="2" y="9" width="12" height="6" fill="currentColor"/>

Does this still look good in 1x?

> Source/WebInspectorUI/UserInterface/Views/AlignmentEditor.css:30
> +    padding: 2px;

IMO this feels a bit too roomy horizontally.  What does it look like if you did `padding: 2px 1px`?

Along these lines, I personally think it looks better when they're all right next to each other (just like a native segmented control <https://developer.apple.com/design/human-interface-guidelines/macos/selectors/segmented-controls/>).  I suppose one major difference is that we don't fill the background when it's selected, but perhaps we could do that (and invert the black box to a white box)?

> Source/WebInspectorUI/UserInterface/Views/AlignmentEditor.css:38
> +    margin-left: 4px;

`margin-inline-start`?  Speaking of, does the order of this flip in RTL?
Comment 6 Nikita Vasilyev 2022-01-10 16:02:57 PST
Created attachment 448807 [details]
Patch
Comment 7 Nikita Vasilyev 2022-01-10 16:03:41 PST
Created attachment 448808 [details]
[Image] After, 1x

(In reply to Devin Rousso from comment #5)
> > Source/WebInspectorUI/UserInterface/Images/AlignContentStretch.svg:4
> > +    <rect x="2" y="9" width="12" height="6" fill="currentColor"/>
> 
> Does this still look good in 1x?

*Now* it does.

Other icons can be improved. Although, most icons on macOS Monterey and Big Sur icons don't look pixel-perfect on 1x either so I don't know how much effort we should put into this going forward.
Comment 8 Nikita Vasilyev 2022-01-10 16:04:20 PST
Created attachment 448809 [details]
[Image] RTL, Dark mode
Comment 9 Devin Rousso 2022-01-10 16:46:28 PST
Comment on attachment 448807 [details]
Patch

r=me
Comment 10 EWS 2022-01-10 23:28:30 PST
Committed r287870 (245914@main): <https://commits.webkit.org/245914@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448807 [details].