[webkit-reviews] review granted: [Bug 214898] Web Inspector: REGRESSION(r255396): Audit: button to exit edit mode in main content area is missing border : [Attachment 405425] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Brian Burg <bburg at apple.com>'s
request for review:
Bug 214898: Web Inspector: REGRESSION(r255396): Audit: button to exit edit mode
in main content area is missing border
https://bugs.webkit.org/show_bug.cgi?id=214898

Attachment 405425: Patch

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




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

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

r=me

> Source/WebInspectorUI/UserInterface/Views/Main.css:239
>  .navigation-item-help > .navigation-bar > .item {

we could also adjust `WI.createNavigationItemHelp` to always expect
`console.assert(navigationItem instanceof WI.ButtonNavigationItem);` so that we
could add `.button` to the CSS since `.text-only` only exists for
`WI.ButtonNavigationItem`

> Source/WebInspectorUI/UserInterface/Views/Main.css:246
> +.navigation-item-help > .navigation-bar > .item:not(.text-only) {

ditto (:239)

> Source/WebInspectorUI/UserInterface/Views/Main.css:250
> +.navigation-item-help > .navigation-bar > .item.text-only {

ditto (:239)

> Source/WebInspectorUI/UserInterface/Views/Main.css:251
> +    border: solid 1px var(--border-color);

It's a bit odd that we have the same CSS property repeated twice exactly as is
for two rules that should both match the same thing.  Perhaps you could add a
comment above it explaining that this is necessary due to the specificity of
`.navigation-bar .item.button.text-only`?