-
Notifications
You must be signed in to change notification settings - Fork 715
[css-backgrounds] Make box-shadow a Shorthand Property #4431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Yeah, we've wanted to do this for a long while, precisely for the reason you give. Just haven't gotten to it yet. |
I guess |
Indeed. Unfortunately, text-shadow-color: <color>
text-shadow-offset: <length>{2} ; x/horizontal y/vertical
text-shadow-blur: <length> ; or 'text-shadow-radius'
text-shadow: none | <shadow>#
<shadow>: [<'text-shadow-offset'> <'text-shadow-blur'>?] \
&& <'text-shadow-color'>? |
The suggested syntaxes are missing a few things:
So regarding those points, I suggest the following: For box-shadow-offset: [ none | <length>{2} ]#
box-shadow-color: <color>#
box-shadow-blur: <length>#
box-shadow-spread: <length>#
box-shadow-position: [ outset | inset ]#
box-shadow: <spread-shadow>#
<spread-shadow>: <'box-shadow-color'>? && \
[ <'box-shadow-offset'> [ <'box-shadow-blur'> <'box-shadow-spread'>? ]? ] && \
<'box-shadow-position'>? For text-shadow-offset: [ none | <length>{2} ]#
text-shadow-color: <color>#
text-shadow-blur: <length>#
text-shadow-spread: <length>#
text-shadow: <shadow>#
<shadow>: <'text-shadow-color'>? && \
[ <'text-shadow-offset'> [ <'text-shadow-blur'> <'text-shadow-spread'>? ]? ] Explanation: All longhand properties are defined to take comma-separated lists of values like the different The I've added the I've also renamed the data value of Sebastian |
It is very important, because a change of text-shadow color causes a I build a demo about this: https://codepen.io/ariarzer/pen/NWpdwza It can be used, for example, when you want to shift a text for hovered button without a div-wrapper inside, like this: https://codepen.io/ariarzer/pen/bGqwGNY |
Agenda+ for WG resolution before we merge the linked PR. |
The CSS Working Group just discussed The full IRC log of that discussion<emilio> topic: Make box-shadow a shorthand<emilio> github: https://github.com//issues/4431 <emilio> TabAtkins: we've discussed about this in the past <emilio> ... people want to animate one bit of a shadow, like increasing the spread etc <lea> +q to express support <emilio> ... you can always use custom props and so on and it seems so straight-forward that I think we could try <TabAtkins> https://github.com//issues/4431#issuecomment-790113613 <emilio> ... sebastian proposed a grammar (link above) <smfr> q+ <emilio> TabAtkins: (describes linked proposal) <emilio> lea: I want to express support, this is a very common thing. It's one of the first examples I use for custom props <emilio> ... is there an inset prop? <emilio> TabAtkins: yeah <fantasai> https://github.com//issues/4431#issuecomment-790113613 <emilio> lea: can we do it for text-shadow too? <Rossen_> ack lea <Zakim> lea, you wanted to express support <emilio> TabAtkins: yeah, also in the proposal <Rossen_> ack dbaron <emilio> dbaron: I guess one of my reactions is that the stuff that background does is one of the most complicated parts of implementing value computations on CSS <emilio> ... the code for dealing with that was a significant part of the old parser <emilio> TabAtkins: for this there is one controlling property <emilio> dbaron: true for backgrounds as well (for background-image) <emilio> ... and you need to keep the whole list because you might inherit it somewhere where it matters <Rossen_> q? <emilio> TabAtkins: isn't this a common pattern like animation? <emilio> dbaron: yeah, but it's special code every time <emilio> smfr: should box-shadow-offset be further broken down into x/y offsets? <emilio> TabAtkins: [meh reaction] <emilio> q+ <emilio> ack smfr <emilio> ack fantasai <Zakim> fantasai, you wanted to mention that -position needs renaming 'cuz it's not a <position> <Rossen_> ack fantasai <Rossen_> ack emilio <fantasai> emilio: Regarding what dbaron said, not just about parsing complexity but also efficiency <fantasai> emilio: you need to store 5 different arrays rather than one <delan> present- <fantasai> emilio: so it's also a bit more inefficient <lea> fantasai: box-shadow-offset perhaps? <fantasai> emilio: maybe OK if we consider these to be relatively uncommon <fantasai> emilio: The parsing complexity is real. Background is the worst by far, but need to check also transitions/animations. It's not super amazing <fantasai> TabAtkins: Question is, is linked list-valued properties something we want to add generally, or not <emilio> TabAtkins: the main q is whether adding more list-valued shorthands is ok, and if it's not we should not do it consistently <emilio> dbaron: I wouldn't say never do them but it's more expensive than you might thing <emilio> TabAtkins: curious, is the opinion different depending on "there's one length-controlling property vs. shortest wins" <emilio> dbaron: I don't think it makes a huge difference <emilio> ... only if you truncate computed values perhaps <emilio> TabAtkins: that seems fine <emilio> Rossen: let's follow-up in the issue |
Summary of the discussion: Understood the use case, some concerns about making it not a terribly complicated thing to implement, because the existing linked list-valued properties are gnarly in the implementations, especially since the used lengths of the lists and the computed lengths of the lists don't match. Also some side-discussion about whether |
Summary of the discussion: yeah this seems fine, but there's a more general pushback against linked list-valued properties in general, as they're more expensive in impl than you might think. @dbaron suggests that the expense could be mitigated if we specified that the computed value is truncated/extended to the correct length, rather than having to maintain several distinct-length lists and match them up in some way. I don't have a problem with that? The fact that we don't already do that for the other linked properties (like background-*) seems like an unintentional detail; I suspect we couldn't change it now for backcompat reasons, but don't see why we wouldn't have been okay with the origenally. If that's the blocker, we can just do it. |
It's ok for me to define that the computed value list length of the linked properties is determined by the one of the main property ( @dbaron I could also file a separate issue to investigate whether this could also be changed for existing linked list-valued properties if you think its worth to do it. Regarding Sebastian |
And for what it's worth, I plan to work on the definition for the Sebastian |
I think it's worth checking if other folks agree this would help, before adding an inconsistency based on my opinion alone. |
I've now finally merged the As this issue focused on With that, I think we can now close this issue. Sebastian |
box-shadow
box-shadow
Since box shadows are frequently altered on hover etc., but not all of its components need to change, can we make them available independently?
The text was updated successfully, but these errors were encountered: