Accept flex-flow's two values in either order - #198
Merged
TylerBrinks merged 1 commit intoJul 23, 2026
Conversation
flex-flow is "<'flex-direction'> || <'flex-wrap'>". The double bar means
the two operands may appear in either order, but the two-value
alternative was built with WithOrder, which requires flex-direction
first, so a reversed pair was rejected outright:
flex-flow: row wrap /* accepted */
flex-flow: wrap row /* dropped, though equally valid */
Use WithAny for the pair. Duplicates and unknown keywords ("row row",
"wrap wrap", "bogus wrap") remain invalid, since "||" still permits at
most one occurrence of each operand.
Reconstructing the shorthand from its longhands still yields only the
direction ("flex-flow: row wrap" read back through StyleDeclaration
gives "row"). That predates this change and is left alone here: the Or
chain picks the first alternative that constructs, and moving the pair
ahead of the single-value alternatives makes an unset operand serialize
its "initial" sentinel instead ("row initial").
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.
Problem
flex-flowis<'flex-direction'> || <'flex-wrap'>(CSS Flexbox 1 §5.1). The double bar means the operands may appear in either order, but the two-value alternative is built withWithOrder, which is theOrderedOptionsConverter:So a reversed pair fails to convert and the declaration is discarded.
Fix
Use
WithAny(theUnorderedOptionsConverter) for the pair.||still permits at most one occurrence of each operand, and that is preserved:row row,wrap wrap,row column,bogus wrapandwrap bogusall remain invalid. Reversed input normalizes to the canonical direction-then-wrap value, soflex-flow: wrap rowyieldsrow wrap.I checked the other
WithOrdercall sites —flex,border-radius,font,background,border-image,cursor,counter-increment/-reset,transform-origin, the ratio and background-position/size compositions — and they all model genuinely ordered grammars.flex-flowwas the only||implemented as ordered.Known gap, left alone
Reconstructing the shorthand from its longhands still yields only the direction —
flex-flow: row wrapread back viaStyleDeclaration.FlexFlowgivesrow. That behaviour predates this change (it reproduces identically onmaster) and is out of scope here.It isn't a one-line follow-up either:
Orpicks the first alternative that succeeds inConstructas well asConvert, so moving the pair ahead of the single-value alternatives fixesrow wrapbut regressesflex-flow: rowto serialize asrow initial, because the unset operand constructs the literalinitialsentinel thatShorthandProperty.Exportwrites. Fixing it properly means suppressing that sentinel during re-serialization, which is a separate change.Tests
14 theory cases in
PropertyTests/FlexPropertyTests.cs: both orders for three direction/wrap pairs (asserting the normalized value and the two longhands), single values, and the invalid combinations.3 fail on
master. The full suite (1263 existing tests) stays green with no existing assertion modified, and all seven target frameworks build with no new warnings.