Implement safe and unsafe overflow-position alignment keywords#952
Open
rit3sh-x wants to merge 5 commits into
Open
Implement safe and unsafe overflow-position alignment keywords#952rit3sh-x wants to merge 5 commits into
safe and unsafe overflow-position alignment keywords#952rit3sh-x wants to merge 5 commits into
Conversation
Add SafeStart/SafeEnd/SafeFlexStart/SafeFlexEnd/SafeCenter on AlignItems and AlignContent with is_safe()/position() helpers, RTL-aware reversed() extension, and a hand-rolled FromCss parser for `safe X` / `unsafe X` that rejects spec-invalid combinations. apply_alignment_fallback now folds the safe modifier into its is_safe flag, so content alignment in grid tracks and flex justify/align-content honor the Start fallback when overflowing. Self-alignment paths accept the new variants but defer the overflow fallback to a follow-up commit.
Implement the Start fallback for "safe" overflow-position keywords on align-self and justify-self in both grid and flex layouts. When an item overflows its alignment container and the alignment value is one of the Safe* variants, the offset is anchored to the logical Start edge to avoid data loss; an item that fits its container keeps its requested position. In grid `align_item_within_area`, overflow is detected from the resolved size and non-auto margins versus the grid area size, and the fallback respects the container `direction` so RTL anchors at the right edge. In flex `align_flex_items_along_cross_axis`, the fallback short-circuits when the cross-axis free space is negative, accounting for `cross_axis_should_reverse` and `is_wrap_reverse` so the Start edge matches the underlying writing-mode-relative start. The three previously-stale `// TODO: Implement safe alignment` comments at the grid track, flex justify-content, and flex align-content sites are removed; the modifier-folding inside `apply_alignment_fallback` already routes Safe* keywords through the Start fallback path on overflow. Add ten integration tests covering the new self-alignment behavior, the content-alignment behavior wired by the previous commit, the no-overflow case, the unsafe (default) overflow regression, the RTL fallback edge, and a multi-line flex `align-content` overflow. The flex absolutely-positioned alignment paths still strip Safe* via `.position()` without invoking the Start fallback; this is left for a follow-up commit.
…ow alignment Add 17 HTML fixtures under test_fixtures/grid/ and test_fixtures/flex/ that exercise the `safe` and `unsafe` overflow-position keywords across container-level (justify-content, align-content) and item-level (justify-self, align-self) alignment in both grid and flex layouts. The fixtures pair overflow and no-overflow geometries so that the Start fallback path and the normal positional path are both verified, and include unsafe-end regressions to prove the default behavior is unchanged for plain end alignment. Run the gentest pipeline against these fixtures to render each in headless Chrome and capture the expected layout into 68 generated XML expectation files (4 variants per fixture: border-box LTR, border-box RTL, content-box LTR, content-box RTL). Register the new entries in tests/xml/mod.rs. The full suite passes — taffy's safe-alignment implementation matches Chrome byte-for-byte on every fixture.
Add a section to docs/style-properties.md that lists the new Safe* variants on AlignItems and AlignContent, describes the overflow fallback to logical Start, and clarifies which position keywords are valid pairings under the spec.
nicoburns
requested changes
May 10, 2026
Member
nicoburns
left a comment
nicoburns
left a comment
There was a problem hiding this comment.
@rit3sh-x This generally looks quite good.
I feel like the "struct containing enum and separate bool" approach might be better, as it would avoid all the unreachable!'s and extra match cases in the current implementation. Perhaps we could even use an enum AlignmentSafety { Safe, Unsafe } instead of bool, because I think there is also an AlignmentSafety::Auto which we may want to implement at some later point.
There are also a few places in the code that keep around as is_safe constant/parameter that doesn't make sense if is_safe is coming in via a style. But I think they might disappear in a refactor to use a boolean / separate enum anyway.
I'm not too worried about ergonomics for consumers of Taffy, as basically every consumer is a framework/toolkit that's converting from another format. I don't think many people are exposing Taffy types directly to end-users.
- Drop dead `is_safe` parameter from `apply_alignment_fallback`; infer it from the alignment style itself. - In `align_item_within_area` (grid) and `align_flex_items_along_cross_axis` (flex), mutate `alignment_style` to `Start` on safe + overflow and reuse the existing match, removing a duplicated RTL branch. - Add `Safe*` arms to `benches/yoga_helpers.rs` so the bench crate builds on CI; Yoga has no overflow-position equivalent, so they map to `unimplemented!()` like the other unsupported variants.
Author
|
@nicoburns I’ve pushed a new commit addressing the concerns you mentioned. Please take a look when you get a chance. |
Author
|
@nicoburns any updates? |
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
2 participants
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.
Summary
Implements the CSS
safe/unsafeoverflow-position keywords for all alignment properties in Flexbox and CSS Grid. Closes #550.safefalls back to logicalStartwhen the alignment subject overflows its container, preventing data loss at the start edge. Existing variants are unchanged and remain equivalent tounsafe.Changes
SafeStart,SafeEnd,SafeFlexStart,SafeFlexEnd,SafeCenteronAlignItemsandAlignContent(aliasesAlignSelf/JustifyItems/JustifySelf/JustifyContentinherit them). Addedis_safe()andposition()helpers.AlignContent::reversed()extended for RTL pairing.FromCssforsafe <position>andunsafe <position>. Rejects spec-invalid combinations (safe stretch,safe baseline,safe space-*).apply_alignment_fallbackfolds theSafe*modifier into itsis_safeflag, so grid-track, flexjustify-content, and flexalign-contentare wired through automatically. Self-alignment paths (align_item_within_areafor grid,align_flex_items_along_cross_axisfor flex) explicitly apply the Start fallback on overflow, RTL- and wrap-reverse-aware.docs/style-properties.md.Tests
reversed(), parser).align-content.4500 tests pass, 0 failures.
Known limitation
The two absolutely-positioned flex paths in
compute/flexbox.rsstripSafe*via.position()but do not apply the Start overflow fallback. MarkedTODO (safe alignment)in code. Happy to land as a follow-up.Test plan
cargo build --all-featurescargo test --all-featurescargo doc --all-features --no-depscargo run -p gentest— 68 new fixtures match Chrome byte-for-byteSafe*-variants approach over asafe: boolfield