-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Roll up css.properties.accent-color.maintains_contrast into parent
#26605
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
Conversation
|
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
|
This was on the agenda for the BCD call today. Since we lack guidelines for this sort of thing, it made it hard to come to a decision. So I'm going to do some backformation here: what would the guidelines look like to accept this change? I think there would have to be two: one to justify the partial and one to justify removing the behavioral subfeature. I'd be interested in @Elchi3 or @caugner reviewing this, to figure out if I've even applied my own guidelines properly. 😄 Hypothetical guidelinesPartial implementationYou must set
Examples of developer-observable evidence of support includes:
Behavioral subfeaturesWhen not otherwise directed to do so by another guideline (such as
Behavioral subfeatures are rare. Do not create a subfeature when any of these conditions are met:
In such cases, consider using notes (or partial implementations) on the parent feature instead. |
|
@ddbeck LGTM overall. Two notes:
PS: Would exposure meet the "indistinguishable" condition (cf. this PR)? |
|
This PR about Otherwise, wouldn't it suggest to remove This also had me thinking that is non-trivial to look up when a feature was introduced in the spec (although the research would be more justified if we recorded a link to the spec commit/PR). |
|
Regarding all the wording stuff: I wrote the guidelines very quickly. If I opened a PR with this, then I would expect to do some work to improve the writing here, in general.
Yeah, you'd pretty much have to keep them, or at least for a long time (maybe 5 years?). I suppose spelling this out would be necessary. To make it possible to get rid of historic behavioral subfeatures, you'd have to invert the data: describe the old behavior as deprecated (or non-standard or both). For example, if you had a
Yes, definitely. If I feature detected
For that feature, I'd suggest at this point to use separate subfeatures for each of the shorthand values (i.e., treat them like any other value for the property and list them one by one). I'm not sure I'd forbid |
|
What would probably help when documenting these guidelines is to provide a couple of real positive and negative examples for both partial implementation, and behavioral subfeatures. As for this PR, I think we can go ahead and merge, and maybe we can continue the guidelines conversation in a BCD discussion, or directly in a PR adding the guidelines? |
Summary
This undoes the structural changes represented by #26493 and the follow-up #26518, but keeps the data as
partial_implementation.I suspect we'll wish to discuss this at the next BCD meeting—before merging—since it touches on (so far minimal) guidelines for
partial_implementationand when to use it.Test results and supporting details
Roughly, the support details are the previous two PRs.
I think there was perhaps a case to be made for Safari being alone as a weird, but mostly functional outlier. But the sum of the two PRs are such that we have this story:
accent-colorhas 3 distinct behaviors for certain color values (Firefox and deskop Chrome; Chrome for Android; Safari).That is to say,
accent-color, taken as a whole, fails the contrast test incompatibly. I think it's easier to tell this story in the support for the property overall than it is to represent this as a behavioral subfeature.That's the main story. There's a side story here that we typically use behavioral subfeatures to represent later additions to existing features or behaviors that are, by specification, discretionary between implementations (that is, "should" not "must"). Here we have behavior required by specification text ("must maintain contrast") that has existed since well before the first implementation (w3c/csswg-drafts@37aeebb). I think the use of a behavioral subfeature here breaks with convention, if not formal practice.
Related issues
css.properties.accent-colorbe partial or not? #26073accent-colorcontrast for legibility #26493accent-colorcontrast for legibility #26518