[Webkit-unassigned] [Bug 217447] Web Inspector: update styles to use CSS properties with neutral directionality

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 27 18:16:37 PDT 2020


https://bugs.webkit.org/show_bug.cgi?id=217447

--- Comment #8 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 412457
  --> https://bugs.webkit.org/attachment.cgi?id=412457
Update styles to use CSS properties with neutral directionality

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

Almost there!  Only a few issues :)

Also, I think you missed one change in `Source/WebInspectorUI/UserInterface/Views/DOMTreeContentView.css` :P

> Source/WebInspectorUI/UserInterface/Views/AuditNavigationSidebarPanel.css:82
> +body .content-view.tab.audit .content-view .reference-page-link-container {

`body` shouldn't be necessary anymore, so please merge this with the above rule.  We used `body` as a way of representing "this is the direction of the entire page" rather than on some other node/selector (which would've been "this is the direction of this one component").

> Source/WebInspectorUI/UserInterface/Views/BezierEditor.css:53
> +    border-inline: 4px;
> +    border-inline-style: solid;
> +    border-inline-color: transparent;

`border-inline: 4px solid transparent`

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineOverviewGraph.css:49
> +    inset-inline-end: var(--cpu-timeline-overview-graph-legend-offset-end);

We can get rid of this variable.  We used to have a rule that for LTR/RTL values, use a variable defined on a rule with the same selector after the `body[dir=*]` so that the value didn't have to be repeated twice.  Anywhere that you see this pattern in the changes you've made so far (you can easily spot this just by searching for the variable and seeing if it's used elsewhere), please just remove the variable.

> Source/WebInspectorUI/UserInterface/Views/HierarchicalPathComponent.css:110
> +    inset-inline-start: var(--path-component-select-offset-start);
> +    inset-inline-end: var(--path-component-select-offset-end);

`inset-inline`

> Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.css:-48
> -body[dir=ltr] .content-view.shader-program > .shader.fragment,
> -body[dir=rtl] .content-view.shader-program > .shader.vertex:not(.shares-vertex-fragment-shader) {
> -    width: calc(50% + 1px);

I don't think we want to remove this.
```
    .content-view.shader-program > .shader.compute,
    .content-view.shader-program > .shader.vertex {
        inset-inline-start: 0;
    }

    .content-view.shader-program > .shader.compute,
    .content-view.shader-program > .shader.vertex.shares-vertex-fragment-shader,
    .content-view.shader-program > .shader.fragment {
        inset-inline-end: 0;
    }

    body[dir=ltr] .content-view.shader-program > .shader.vertex:not(.shares-vertex-fragment-shader),
    body[dir=rtl] .content-view.shader-program > .shader.fragment {
        width: calc(50% - 1px);
    }

    body[dir=ltr] .content-view.shader-program > .shader.fragment,
    body[dir=rtl] .content-view.shader-program > .shader.vertex:not(.shares-vertex-fragment-shader) {
        width: calc(50% + 1px);
    }
```
IIRC, the `1px` in the `calc` is used to provide space for the divider line between the vertex shader editor and fragment shader editor.

> Source/WebInspectorUI/UserInterface/Views/SingleSidebar.css:40
> +body .single-sidebar.trailing > .resizer,
> +body .single-sidebar.leading > .resizer {
> +    inset-inline: -3px;

This isn't quite right.  We don't want to set both `left` and `right` for both.
```
    .single-sidebar.trailing > .resizer {
        inset-inline-start: -3px;
    }

    .single-sidebar.leading > .resizer {
        inset-inline-end: -3px;
    }
```

> Source/WebInspectorUI/UserInterface/Views/SpringEditor.css:81
> +body .spring-editor > .spring-timing::before,
> +body .spring-editor > .spring-timing::after {
> +    inset-inline: var(--spring-editor-timing-pseudo-offset-start) var(--spring-editor-timing-pseudo-offset-end);

This isn't quite right.  We don't want to set both `left` and `right` for both.
```
    .spring-editor > .spring-timing::before {
        inset-inline-start: 0;
    }

    .spring-editor > .spring-timing::after {
        inset-inline-end: 0;
    }
```

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.css:34
> +body .timeline-overview > :matches(.navigation-bar.timelines, .tree-outline.timelines) {

please merge this with the previous rule

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:59
> +    border-inline: var(--record-bar-segment-border-start) var(--record-bar-segment-border-end);

this should be moved to `.timeline-record-bar > .segment`

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.css:268
> +body .timeline-ruler > .shaded-area.left,
> +body .timeline-ruler > .shaded-area.right {
> +    inset-inline: 0;

This isn't quite right.  We don't want to set both `left` and `right` for both.
```
    .timeline-ruler > .shaded-area.left {
        inset-inline-start: 0;
    }

    .timeline-ruler > .shaded-area.right {
        inset-inline-end: 0;
    }
```

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201028/b9ed63ba/attachment.htm>


More information about the webkit-unassigned mailing list