amp-selector - CSS Outline is too Specific

[camelburrito]

For example, in AMP start, I needed to make my selector like this to make the override possible.,

body .www-component-anchors .www-component-anchor[option][selected]

we should split up the selectors into smaller (less specific ones) to make it easier to override - we would need to follow the same principle in all our components to make overriding more simple.

[camelburrito]

CC @cramforce @aghassemi

[cramforce]

Would this change be backward compatible?

[camelburrito]

Yes

[aghassemi]

Question, why do we have the outline in the first place? I ran into this the other day and was surprised to see the outline, initially thought something with my CSS is broken until I inspected the element and noticed the outline is coming from the amp-selector.

My opinion is to remove the outline all together and keep amp-selector purely a behavior component without any style.

[camelburrito]

That was what I thought too , but UX + few others(while reviewing) wanted a default CSS to guide authors. Which is not a bad idea.

[dvoytenko]

All, TBH, it was me who insisted on the outline. And I still think it's the right thing to do, though would be happy to discuss. Obviously, CSS specificity is a pain and the only way I see it resolve would be via an internal class, as much as I'd hate that. A user would still be invited to override via x[selected] or such to preserve semantics.

[camelburrito]

@dvoytenko - I think @abeck000 and @ericlindley-g were on board with that too. Its probably not just you.

I think specificity can be solved by writing simple selectors in this case (it was my mistake)

amp-selector:not([disabled]) [option][selected]:not([disabled]) {
  outline: solid 1px rgba(0,0,0,0.7);
}

could have been

amp-selector [option][selected] {
  outline: solid 1px rgba(0,0,0,0.7);
}

amp-selector [option][disabled],
amp-selector[disabled] [option] {
  outline: none;
}

would have made it easier to override.

I general we should make it a practice to write easily overridable selectors.

[ericlindley-g]

Though the case is somewhat ambiguous, I also still think the current visual styling is the right thing to do, but am happy to discuss. Agree that changes to make it easier to override would be good, given they're fully backward-compatible

[cramforce]

Ick, yes, lets simplify that selector.

…

On Tue, Feb 14, 2017 at 12:56 PM, ericlindley-g ***@***.***> wrote: The case is somewhat ambiguous, I also still think the current visual styling is the right thing to do, but am happy to discuss. Agree that changes to make it easier to override would be good, given they're fully backward-compatible — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7534 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFeT4XDZdWuCWaBpXOIaExVb8GqJt9kks5rchUagaJpZM4MABrS> .

[dvoytenko]

@camelburrito I think this is still too hard to override. Here's the goal, we need to have selectors that can be overridden as simply as:

.myoption[selected] {
  ...
}
.myoption[selected][disabled] {
  ...
}

[camelburrito]

correct it should be

amp-selector [option][selected] {
  outline: solid 1px rgba(0,0,0,0.7);
}

amp-selector [selected][disabled],
amp-selector[disabled] [selected] {
  outline: none;
}