Repository navigation
Conversation
These are the pseudo-elements of css-overflow-5's scroll navigation controls, the same spec section the existing SCROLL_NAVIGATION_CONTROLS flag already gates :target-current, :target-before and :target-after behind, so they are gated the same way. Also allows the three :target-* pseudo-classes after ::scroll-marker. Selectors 4 permits only the user action pseudo-classes after a pseudo-element, and css-overflow-5 defines these three specifically to match scroll markers, so ::scroll-marker:target-current has to parse. That goes through a new AFTER_SCROLL_MARKER parsing state, mirroring how AFTER_WEBKIT_SCROLLBAR already handles the same kind of exception. a::before:target-current stays an error.
Author
|
Re-verified on master (c6a0c3c) today, through a node binding built from it. All three still warn while being emitted verbatim: The pass-through is why this is advisory rather than breaking, and it is also why it is easy to leave: a build that treats warnings as errors fails on valid CSS Overflow 5, and one that does not carries a warning per carousel selector per build forever. Still applies cleanly and the suite is green on it. Happy to rebase whenever it is useful. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1177. Part of #1184.
Problem
::scroll-marker,::scroll-marker-groupand::scroll-button()are not recognized, so a stylesheet using the CSS Overflow 5 carousel pattern gets threeis not recognized as a valid pseudo-elementwarnings on correct CSS. They parse asCustomand pass through, so only the diagnostic is wrong.The three
:target-*pseudo-classes from the same spec section landed in #1185, and theSCROLL_NAVIGATION_CONTROLSflag added there already points at the section that defines these. This fills in the rest of it.Changes
The three pseudo-elements, behind the existing flag.
ScrollMarker,ScrollMarkerGroupandScrollButton { direction }are real variants with their own serialization, gated onSCROLL_NAVIGATION_CONTROLSexactly as:target-currentis. Nothing changes with the flag off.::scroll-button()takes'*' | <scroll-button-direction>.ScrollButtonDirectioncarries the ten keywords the spec defines (up,down,left,right,block-start,block-end,inline-start,inline-end,prev,next) plus*, matched case-insensitively and serialized canonically, so::SCROLL-BUTTON(BLOCK-START)prints as::scroll-button(block-start). An unknown keyword is aSelectorError::UnexpectedIdentrather than a silentCustom.A second fix that the first one needs:
::scroll-marker:target-currentwas rejected. Selectors 4 allows only the user action pseudo-classes after a pseudo-element, and css-overflow-5 defines the three:target-*pseudo-classes specifically to match scroll markers, so this is the shape the feature is actually written in. It went through a newAFTER_SCROLL_MARKERparsing state and aNonTSPseudoClass::is_valid_after_scroll_markerwith a default ofis_user_action_state(), which mirrors howAFTER_WEBKIT_SCROLLBARandis_valid_after_webkit_scrollbaralready handle the same kind of spec-defined exception. Both trait methods are additive with defaults, so no other implementor changes.The narrow shape is deliberate. A blanket widening was the first thing I tried and it turned #1185's
a::before:target-currenterror test green, which is the wrong answer; keying on the preceding pseudo-element keeps that an error, and its test is unchanged and still passing.compat.rsis generated, not hand-edited. ThreemdnFeaturesentries inscripts/build-prefixes.js, thennode scripts/build-prefixes.js. All three are Chrome 135, no Firefox or Safari. The only change the run produced was those features, so the diff carries no unrelated churn.Testing
cargo test -p lightningcsspasses, 120 of 120, with the new cases intest_selectors: each pseudo-element,*and a flow-relative keyword, the case-insensitivity round trip,::scroll-marker:target-current,a::before:target-currentstill erroring, an unknown direction erroring, and both pass-through cases with the flag off so the no-flag behaviour is pinned.Two things worth flagging rather than hiding.
cargo test -p parcel_selectorshas one failure,parser::tests::test_parsingassertingparse("foo::details-content").is_ok(); it fails identically on an unpatched checkout of this branch's base, so it is pre-existing. Andcargo test --workspacecannot linklightningcss_nodeoutside a Node build in my environment (undefined napi symbols), which is unrelated to this diff.Downstream
Bun ports this parser, and its table trails yours: oven-sh/bun#41120, where oven-sh/bun#41122 is adding the four entries you already have. They are holding the scroll-marker family at parity with lightningcss deliberately, so this reaching upstream is what carries it to them.