Repository navigation
feat: you can now supply your own traversal - #24
Merged
Merged
Conversation
`order` takes an `OrderFn` and replaces the preset's. This is the seam the layered design always left open — which dot leads is one function of lattice position — finally reachable from an option rather than only from someone writing their own renderer. IT IS FREE IN BOTH RENDERERS, which is the asymmetry worth stating. The CSS host derives each cell's `animation-delay` from the same traversal value, so a custom traversal costs zero per-frame JS there too. A custom ENVELOPE would not be free: the CSS animation is one shared `@keyframes` published per preset, so a per-caller brightness curve needs the stylesheet's cache key and the animation name to change. Hence traversal only, and a test that asserts the CSS host's `--botui-o` matches — a traversal the engine forgot to apply there would produce two renderers that disagree, which is the one thing this library is arranged to make impossible. TWO THINGS THE TYPE HAS TO GET RIGHT, and the second is a trap: - `order` must be omitted from `ResolvedDotMatrixOptions`'s `Required` alongside `stagger`. Left in, the whole resolved-options type demands a traversal nobody supplied, and `DEFAULTS` would need a default traversal — which is one more thing silently overriding the caller. `order` stays `undefined` until someone passes one, and `resolveField` is where the preset's traversal takes over. - `OrderFn`'s fifth argument stays optional, so an existing four-argument traversal is still a valid `OrderFn` and a custom traversal with no sense of direction is not forced to accept one. THE README EXAMPLE IS EXECUTED BY A TEST. A README snippet that does not compile is worse than none — it is the first thing a reader runs and the thing they judge the API by. The test runs the snippet verbatim, on 1×1 / 1×7 / 7×1 grids where a pasted traversal divides by zero, and asserts that it really ACCELERATES: an easing that did nothing would still run and still look plausible, so the prose's claim has to be checked against the numbers it makes. The snippet is the accelerating chevron, which is one of the two motions asked for — so "accelerating arrows" turned out not to need a new preset at all, only a traversal that eases along its own axis. A centre point for the concentric orders is the same shape of answer: it is geometry the traversal computes, so `order` covers it without a new option. IT IS A FUNCTION, SO IT IS NOT COPYABLE, and the playground's copy button leaves it out rather than emitting a prop that would not compile. A component carrying a custom traversal is a component whose motion is code; that is a real constraint of this route and it is stated in both the README and the option's own docs. 260 tests (was 249), oxlint 0, registry ok, both changelog ledgers.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 31 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Why
The Traversal layer has always been one function of lattice position — the seam the whole
layered design leaves open. It was reachable only by writing your own renderer. Now it is an
option.
Change outline
It is free in both renderers — the CSS host derives each cell's
animation-delayfromthe same traversal value, so a custom traversal costs zero per-frame JS there too.
That asymmetry is the design: a custom envelope would not be free, because the CSS
animation is one shared
@keyframespublished per preset. So: traversal only, with a testasserting the CSS host's
--botui-omatches. A traversal the engine forgot to apply therewould produce two renderers that disagree — the one thing this library is arranged to make
impossible.
"Accelerating arrows" turned out not to need a new preset at all — only a traversal that
eases along its own axis. A centre point for the concentric orders is the same shape of
answer: geometry the traversal computes, so
ordercovers it without a new option.Special things to note
ordermust be omitted fromResolvedDotMatrixOptions'sRequiredalongside
stagger— left in, the resolved type demands a traversal nobody supplied andDEFAULTSwould need a default traversal, which is one more thing silently overriding thecaller.
OrderFn's fifth argument stays optional, so an existing four-argument traversalis still valid and one with no sense of direction is not forced to accept it.
than none — it is the first thing a reader runs. The test runs it verbatim, on 1×1 / 1×7 /
7×1 grids where a pasted traversal divides by zero, and asserts it really accelerates:
an easing that did nothing would still run and still look plausible.
emitting a prop that would not compile. Stated in the README and the option's docs, since
it is a real constraint of this route.
🤖 Generated with Claude Code