WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
223418
Web Inspector: Layout sidebar grid overlay color swatch tooltip shouldn't include "switch format" hint
https://bugs.webkit.org/show_bug.cgi?id=223418
Summary
Web Inspector: Layout sidebar grid overlay color swatch tooltip shouldn't inc...
Patrick Angle
Reported
2021-03-18 00:32:59 PDT
Steps to reproduce: 1. Inspect a page with a CSS grid 2. Hover over a grid overlay swatch in the Layout sidebar Results: Tooltip will read: ``` Click to select a color Shift-click to switch color formats ``` Expected result: Tooltip should read: ``` Click to select a color ``` and not reference switching formats, which is not applicable in this context.
Attachments
Patch
(5.18 KB, patch)
2021-04-01 08:05 PDT
,
Razvan Caliman
no flags
Details
Formatted Diff
Diff
Patch
(3.01 KB, patch)
2021-04-01 10:59 PDT
,
Razvan Caliman
no flags
Details
Formatted Diff
Diff
Patch
(4.11 KB, patch)
2021-04-02 05:25 PDT
,
Razvan Caliman
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2021-03-25 00:33:14 PDT
<
rdar://problem/75825793
>
Razvan Caliman
Comment 2
2021-04-01 08:05:14 PDT
Created
attachment 424896
[details]
Patch
Patrick Angle
Comment 3
2021-04-01 09:15:30 PDT
Comment on
attachment 424896
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=424896&action=review
> Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js:184 > if (this._allowShiftClickColor())
Just noticing there is a check if shift-clicking should even be allowed, and if it is false we would get the string we wanted. Maybe instead of passing a title into the swatch we should support directly enabling/disabling shift-click, since _swatchElementClicked also has a check for shift-clicking being allowed?
Blaze Burg
Comment 4
2021-04-01 09:54:32 PDT
Comment on
attachment 424896
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=424896&action=review
>> Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js:184 >> if (this._allowShiftClickColor()) > > Just noticing there is a check if shift-clicking should even be allowed, and if it is false we would get the string we wanted. Maybe instead of passing a title into the swatch we should support directly enabling/disabling shift-click, since _swatchElementClicked also has a check for shift-clicking being allowed?
+1, it makes more sense to me, to have the swatch object itself be readonly.
Razvan Caliman
Comment 5
2021-04-01 10:59:00 PDT
Created
attachment 424912
[details]
Patch
Blaze Burg
Comment 6
2021-04-01 11:21:05 PDT
Comment on
attachment 424912
[details]
Patch r=me The logic looks fine to me. I am not fond of the existing _allowShiftClickColor name. Something more like allowsChangingColorFormats would be more about the semantics of what is possible or not, rather than how it is currently exposed.
Patrick Angle
Comment 7
2021-04-01 11:23:22 PDT
Comment on
attachment 424912
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=424912&action=review
> Source/WebInspectorUI/UserInterface/Views/InlineSwatch.js:128 > + set shiftClickColorEnabled(value) {
NIT: `{` on next line
Razvan Caliman
Comment 8
2021-04-02 05:25:11 PDT
Created
attachment 425010
[details]
Patch Carry over R+; Update method name, fix style issue
EWS
Comment 9
2021-04-06 00:28:35 PDT
Committed
r275493
: <
https://commits.webkit.org/r275493
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 425010
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug