Bug 223418 – Web Inspector: Layout sidebar grid overlay color swatch tooltip shouldn't include "switch format" hint

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Patrick Angle]

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.

[Radar WebKit Bug Importer]

<rdar://problem/75825793>

[Razvan Caliman]

Created attachment 424896 [details]
Patch

[Patrick Angle]

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 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]

Created attachment 424912 [details]
Patch

[Blaze Burg]

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 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]

Created attachment 425010 [details]
Patch

Carry over R+; Update method name, fix style issue

[EWS]

Committed r275493: <https://commits.webkit.org/r275493>

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