RESOLVED FIXED 225433
Web Inspector: Layout panel "Grid Overlays" main checkbox has dead space (no interaction) between checkbox and label text
https://bugs.webkit.org/show_bug.cgi?id=225433
Summary Web Inspector: Layout panel "Grid Overlays" main checkbox has dead space (no ...
Patrick Angle
Reported 2021-05-05 21:28:02 PDT
Created attachment 427848 [details] Screenshot illustrating issue The attached screenshot shows a small area where clicking on the "Grid Overlays" checkbox/label does not toggle the item. This is different from the below item for a single overlay, where that same gap space does toggle the item. System checkboxes treat that space as interactive, so our header's behavior is incorrect here.
Attachments
Screenshot illustrating issue (149.70 KB, image/png)
2021-05-05 21:28 PDT, Patrick Angle
no flags
Patch (10.89 KB, patch)
2021-05-06 15:10 PDT, Nikita Vasilyev
hi: review+
hi: commit-queue-
[Video] With patch applied (2.07 MB, video/quicktime)
2021-05-06 15:10 PDT, Nikita Vasilyev
no flags
Patch (11.19 KB, patch)
2021-05-10 10:58 PDT, Nikita Vasilyev
ews-feeder: commit-queue-
Radar WebKit Bug Importer
Comment 1 2021-05-05 21:28:55 PDT
Patrick Angle
Comment 2 2021-05-05 21:36:31 PDT
Comment on attachment 427848 [details] Screenshot illustrating issue This diagram is technically wrong, because the "Grid Overlays" hit box doesn't extend this far to the right, but the area of issue is still accurate.
Nikita Vasilyev
Comment 3 2021-05-06 15:10:23 PDT
Nikita Vasilyev
Comment 4 2021-05-06 15:10:52 PDT
Created attachment 427942 [details] [Video] With patch applied
Devin Rousso
Comment 5 2021-05-07 12:37:10 PDT
Comment on attachment 427941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427941&action=review r=me with some minor adjustments > Source/WebInspectorUI/UserInterface/Views/CSSGridSection.css:69 > +.css-grid-section input[type="checkbox"] { Can we be more specific about this selector? ``` .css-grid-section :is(.setting-editor, .node-overlay-list-item-container) input[type="checkbox"] ``` > Source/WebInspectorUI/UserInterface/Views/CSSGridSection.js:90 > + labelInner.append(WI.UIString("Grid Overlays", "Page Overlays @ Layout Sidebar Section Header", "Heading for list of grid nodes")); `.textContent = ` > Source/WebInspectorUI/UserInterface/Views/SettingEditor.css:31 > + padding-inline-start: 5px; This used to also have `5px` of padding on the right too inside `WI.CSSGridSection`. Do we not want that anymore? Also, this is super NIT, but I personally think `4px` looks visually better 😅 > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.css:108 > +.content-view.tab.settings > .settings-view > .container > .editor-group > .setting-editor input { > font-size: inherit; I feel like we should move more of the generic styles like this to `Source/WebInspectorUI/UserInterface/Views/SettingEditor.css`. The only stuff that should be in this file are the styles that are necessary to make the Settings Tab look good.
Nikita Vasilyev
Comment 6 2021-05-10 10:57:39 PDT
Comment on attachment 427941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427941&action=review >> Source/WebInspectorUI/UserInterface/Views/SettingEditor.css:31 >> + padding-inline-start: 5px; > > This used to also have `5px` of padding on the right too inside `WI.CSSGridSection`. Do we not want that anymore? I don't think we do. I just checked macOS - it doesn't arbitrary increase the clickable area to the right of the label text. I added 5px padding initially to separate label text and the color picker swatch, and it somehow creeped in to checkbox labels that don't have color picker swatches.
Nikita Vasilyev
Comment 7 2021-05-10 10:58:06 PDT
EWS
Comment 8 2021-05-10 12:33:24 PDT
Committed r277284 (237548@main): <https://commits.webkit.org/237548@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428184 [details].
Note You need to log in before you can comment on or make changes to this bug.