Web Inspector: Styles Redesign: properties should never be semitransparent or crossed out while editing

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Created attachment 329309 [details]
[Image] Bug

Non-inherited properties of inherited rules are semitransparent (opacity: 0.5).
When editing, the text is gray and text selection color is too dim. Newly added properties
of inherited rules are semitransparent, too.

All properties should be opaque when editing.

[Radar WebKit Bug Importer]

<rdar://problem/36038813>

[Nikita Vasilyev]

Created attachment 329312 [details]
[Image] Edited properties should not be crossed out

[Devin Rousso]

Created attachment 329464 [details]
Patch

[Nikita Vasilyev]

Created attachment 329547 [details]
[Image] inline vs inline-block

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

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:-54
> -.spreadsheet-style-declaration-editor .value.editing {
> -    display: inline-block;
> -    margin-right: 3px;
> -}

Initially, I made values inline, but after one of the design meetings two months ago we decided to change it to inline-block.

Text selection looks better with inline-block. As a downside, value element may move when you start editing it.

I'd prefer to keep it inline-block.

We should hide ";" while editing values.

[Nikita Vasilyev]

Comment on attachment 329464 [details]
Patch

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

Looks good overall.

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:113
> -.spreadsheet-style-declaration-editor .property.not-inherited {
> +.spreadsheet-style-declaration-editor .property.not-inherited .content > * {

I generally try to avoid using * in CSS, but I think it's fine here.

[Nikita Vasilyev]

Comment on attachment 329464 [details]
Patch

r- because I don't want `display: inline-block` to be removed.

[Nikita Vasilyev]

Devin, do you mind if I finish this one? There was no activity here for a month. I'll post an updated patch tonight unless you want to finish it yourself.

[Devin Rousso]

(In reply to Nikita Vasilyev from comment #7)
> Devin, do you mind if I finish this one? There was no activity here for a
> month. I'll post an updated patch tonight unless you want to finish it
> yourself.
Go for it.  I'm in the midst of Syllabus week right now, so things are a bit hectic.  Sorry for the delay 😅

[Nikita Vasilyev]

Created attachment 331296 [details]
Patch

[Nikita Vasilyev]

Created attachment 331298 [details]
[Image] With patch applied

[Devin Rousso]

Comment on attachment 331296 [details]
Patch

r=me

One thing that I noticed when I was working on this was that when you edit the value, it shifts the semicolon around.  This is because of the `margin-right: 3px;` in the `.spreadsheet-style-declaration-editor .value.editing` rule.  If you remove that, the semicolon doesn't shift anymore, but it now appears "above" the shadow.  I think the solution here is to move the editing element "above" the rest of the content while it is being modified.

    .spreadsheet-style-declaration-editor .value.editing {
        display: inline-block;
        position: relative;
        z-index: 1;
    }

[Nikita Vasilyev]

Created attachment 331299 [details]
Patch

(In reply to Devin Rousso from comment #11)
> One thing that I noticed when I was working on this was that when you edit
> the value, it shifts the semicolon around.  This is because of the
> `margin-right: 3px;` in the `.spreadsheet-style-declaration-editor
> .value.editing` rule.  If you remove that, the semicolon doesn't shift
> anymore, but it now appears "above" the shadow.  I think the solution here
> is to move the editing element "above" the rest of the content while it is
> being modified.
> 
>     .spreadsheet-style-declaration-editor .value.editing {
>         display: inline-block;
>         position: relative;
>         z-index: 1;
>     }

This looks good, thanks!

[WebKit Commit Bot]

Comment on attachment 331299 [details]
Patch

Clearing flags on attachment: 331299

Committed r226939: <https://trac.webkit.org/changeset/226939>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.