1553127 - [Inactive CSS] Display a warning when `place-*` is used incorrectly

Status: VERIFIED FIXED

(DevTools :: Inspector: Rules, enhancement, P2)

Description

[Patrick Brosset <:pbro>]

place-self, place-items and place-content can only be used on or in grid/flex layout containers (at least today), so warnings need to be displayed in the inactive CSS tooltips when they are used elsewhere.
We currently display warnings for the align-* and justify-* properties, but we should do the same for place-*.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: image.png

Correct me if I'm wrong, but I think place-self can be used on flex and grid items.
What's interesting though, is that on flex items, the second value will be ignored (since justify-self can't be used on flex item), but we don't have a UI for that on the inspector UI.

Maybe we could have something like that to start with?

place-self: ▼ center center
    align-self: center;
    justify-self: center ⓘ;

We could also have it directly next to the second value, greyed out? I guess that might be confusing for people

place-self: ▼ center center ⓘ 
    align-self: center;
    justify-self: center ⓘ;

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1553127 - Display a warning when `place-self` is used on non flex/grid items. r=pbro.

We take this as an opportunity to add tests for align-self as well.
This requires the test to change a bit so we can create more than one
element in order to test the inactive property helper on grid/flex item
(i.e. with a parent flex/grid container).
This is done by providing a createTestElement function in the test case,
which takes the document as a parameter, creates whatever nodes it need,
and returns an object with 2 properties:

• elementToInsert will be the node that will be inserted in the document
• elementToTest will be the node we check isPropertyUsed on.

Comment 3

[Patrick Brosset <:pbro>]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)

Correct me if I'm wrong, but I think place-self can be used on flex and grid items.
What's interesting though, is that on flex items, the second value will be ignored (since justify-self can't be used on flex item)
Good point, that is correct.

Maybe we could have something like that to start with?

place-self: ▼ center center
    align-self: center;
    justify-self: center ⓘ;

We could also have it directly next to the second value, greyed out? I guess that might be confusing for people

place-self: ▼ center center ⓘ 
    align-self: center;
    justify-self: center ⓘ;

So, as an initial thing, I would probably not worry about this case yet. I would display the warning when place-self is used on something where it is not supported at all. But if it is supported, even partially, lile on flex items, I wouldn't display the warning.
In this initial phase of this feature, I'd prefer if we added warnings not too eagerly until we are comfortable with how the feature works, and how maintainable it is on the long run.

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1553127 - Display a warning when place-items or place-content is used on non flex/grid container. r=pbro.

Depends on D34714

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ce8678568c4
Display a warning when `place-self` is used on non flex/grid items. r=pbro.
https://hg.mozilla.org/integration/autoland/rev/751af6638b4b
Display a warning when place-items or place-content is used on non flex/grid container. r=pbro.

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9ce8678568c4
https://hg.mozilla.org/mozilla-central/rev/751af6638b4b

Comment 7

[Cristian Fogel [:cfogel]]

Confirming with 69.0b4 that the place-* (self, items, content) rules are properly marked as inactive.