Bug 202253 – Web Inspector: Canvas: shader type header is white in dark mode

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

[Devin Rousso]

Created attachment 379606 [details]
[Image] Screenshot of Issue

.

[Devin Rousso]

Created attachment 379608 [details]
Patch

[Nikita Vasilyev]

Comment on attachment 379608 [details]
Patch

It looks like <header> always has only one child.

Can this markup

    <header>
        <div class=shader-type">...</div>
    </header>

be replaced with

    <header class=shader-type">...</header>

?

[Devin Rousso]

(In reply to Nikita Vasilyev from comment #2)
> Comment on attachment 379608 [details]
> Patch
> 
> It looks like <header> always has only one child.
I did this so that if we wanted to add additional editing items (e.g. an <input> to change a WebGPU shader pipeline's `entryPoint`), there'd be an easy way to do that.

[Nikita Vasilyev]

Comment on attachment 379608 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.css:65
> +    .content-view.shader-program > .shader > header > * {

I try to avoid selectors ending on `*`. CSS selectors are applied right-to-left, so selectors ending on `*` aren't very effecient.

Looks like for this case `.content-view.shader-program > .shader > header` should be a sufficient selector.

> Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.css:69
> +    .content-view.shader-program > .shader > header > .shader-type {

Ditto. `.content-view.shader-program > .shader > header`.

[Devin Rousso]

Comment on attachment 379608 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.css:65
>> +    .content-view.shader-program > .shader > header > * {
> 
> I try to avoid selectors ending on `*`. CSS selectors are applied right-to-left, so selectors ending on `*` aren't very effecient.
> 
> Looks like for this case `.content-view.shader-program > .shader > header` should be a sufficient selector.

Good point.  I'll merge this with the rule below (same with the light-mode variant).

>> Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.css:69
>> +    .content-view.shader-program > .shader > header > .shader-type {
> 
> Ditto. `.content-view.shader-program > .shader > header`.

See comment #3:
> I did this so that if we wanted to add additional editing items (e.g. an <input> to change a WebGPU shader pipeline's `entryPoint`), there'd be an easy way to do that.

[Devin Rousso]

Created attachment 379844 [details]
Patch

[Nikita Vasilyev]

Comment on attachment 379608 [details]
Patch

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

Looks fine.

> Source/WebInspectorUI/ChangeLog:9
> +        (@media (prefers-color-scheme: dark) .content-view.shader-program > .shader > header > *): Added.

Nit: the changelog is out of date.

>>> Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.css:69
>>> +    .content-view.shader-program > .shader > header > .shader-type {
>> 
>> Ditto. `.content-view.shader-program > .shader > header`.
> 
> See comment #3:

I saw it. I assumed it would go inside of the header.

[Matt Baker]

Comment on attachment 379844 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 379844 [details]
Patch

Clearing flags on attachment: 379844

Committed r250533: <https://trac.webkit.org/changeset/250533>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55860197>