Bug 234036 - Web Inspector: Increase padding around icons in Alignment editor
Summary: Web Inspector: Increase padding around icons in Alignment editor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-12-08 14:36 PST by Nikita Vasilyev
Modified: 2022-01-10 23:28 PST (History)
5 users (show)

See Also:

Attachments
Patch (2.92 KB, patch)
2022-01-10 13:06 PST, Nikita Vasilyev 		no flags Details | Formatted Diff | Diff
[Image] Before (23.02 KB, image/png)
2022-01-10 13:06 PST, Nikita Vasilyev 		no flags Details
[Image] After (22.04 KB, image/png)
2022-01-10 13:07 PST, Nikita Vasilyev 		no flags Details
Patch (3.37 KB, patch)
2022-01-10 16:02 PST, Nikita Vasilyev 		no flags Details | Formatted Diff | Diff
[Image] After, 1x (11.71 KB, image/png)
2022-01-10 16:03 PST, Nikita Vasilyev 		no flags Details
[Image] RTL, Dark mode (27.57 KB, image/png)
2022-01-10 16:04 PST, Nikita Vasilyev 		no flags Details
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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].