Fork anchored region from @ni/fast-foundation - #3025
Draft
m-akinc wants to merge 3 commits into
Draft
Conversation
m-akinc
marked this pull request as ready for review
July 31, 2026 23:02
Member
|
Talked to @m-akinc and emphasized that was looking towards re-implementing with popover api as part of the popover research task and want to avoid multiple back-to-back refactors of a shared low-level component if not necessary. Stated there wasn't an immediate rush so moving to draft. |
rajsite
marked this pull request as draft
July 31, 2026 23:13
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.
Pull Request
🤨 Rationale
To better enable a fix for #3002, we decided to add an API to explicitly activate/deactivate anchored regions. Rather than adding new functionality to the
@ni/fast-foundationpackage, we want to fork the implementation intonimble-componentsfirst, then add the new API there.👩💻 Implementation
index.ts),intersection-service.ts, and (meager) unit testsupdateLayout()implementation into smaller functions that use common logic for both vertical and horizontal updates, rather than separate functions with high levels of duplication.Note to author: do not copy and paste LLM responses as PR descriptions. Do your own technical evaluations. If this PR is to be merged, this should be removed.
I had Sonnet 5 compare the refactor to the original, and it verified the correctness: >I compared `index.ts` against `anchored-region.ts` in the FAST foundation source (`anchored-region.ts`, the `@ni/fast-foundation` fork nimble depends on) method by method. Summary: the refactor is functionally faithful — I found no logic bugs. > >**What changed structurally** >The nimble version collapses FAST's duplicated horizontal/vertical code paths into shared per-axis helpers driven by a new internal `Dimension` type (`{ regionSize, anchorStart, anchorEnd, viewportStart, viewportEnd }`) ... >... >I traced every arithmetic expression (translation math for `start`/`insetStart`/`insetEnd`/`end`/`center`, viewport-lock corrections, available-space formulas, the "enough space vs. threshold vs. locktodefault" branching, and the direction-change reset check) and each one is mathematically identical to the original, just reparameterized. Notably: ... > >**Intentional (non-bug) deviations** >- Nimble uses the global `ResizeObserver`directly, while FAST casts `window` to a custom `WindowWithResizeObserver`/`ResizeObserverClassDefinition` (presumably for polyfill/SSR safety). This matches the convention used elsewhere in nimble-components (text-area/index.ts, tabs/index.ts, anchor-tabs/index.ts), so it's a deliberate, consistent simplification rather than a regression. >- `regionWidth`/`regionHeight` are typed as optional (`string | undefined`) in nimble vs. non-optional (but effectively unset initially) in FAST — a typing improvement, no behavior change. > >No functional deviations were found; the port preserves FAST's positioning algorithm exactly. - (incidental) moved `breadcrumb.foundation.spec.ts` from the root `breadcrumb` directory to the `tests` subdirectory🧪 Testing
Automated tests pass.