[webkit-reviews] review granted: [Bug 211105] Web Inspector: Timelines: Edit button has wrong outline : [Attachment 398543] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 211105: Web Inspector: Timelines: Edit button has wrong outline
https://bugs.webkit.org/show_bug.cgi?id=211105

Attachment 398543: Patch

https://bugs.webkit.org/attachment.cgi?id=398543&action=review




--- Comment #6 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 398543
  --> https://bugs.webkit.org/attachment.cgi?id=398543
Patch

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

r=me

> Source/WebInspectorUI/ChangeLog:18
> +	   Remove dead code. We only have text-only buttons now.

I don't think we should limit ourselves to only having text-only
`WI.RadioButtonNavigationItem`.

If we really do want to do that, we should
 - add `console.assert(this.buttonStyle ===
WI.ButtonNavigationItem.Style.Text);` to the `constructor`
 - override `set buttonStyle` so that it fails if anything other than
`WI.ButtonNavigationItem.Style.Text` is provided

> Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css:69
> +    outline-offset: var(--focus-ring-outline-offset);

Perhaps we can use the CSS variable override approach like `WI.ScopeBar` does.

ButtonNavigationItem.css
```
    .navigation-bar .item.button.text-only:focus {
	outline-offset: var(--outline-offset-override,
var(--outline-offset-default);

	--outline-offset-default: var(--focus-ring-outline-offset);
    }
```

RadioButtonNavigationItem.css
```
    .navigation-bar .item.radio.text-only:focus {
	--outline-offset-override: -1px;
    }
```

This way, selector specificity is irrelevant.