Skip to content

Fork anchored region from @ni/fast-foundation - #3025

Draft
m-akinc wants to merge 3 commits into
mainfrom
users/makinc/forked-anchored-region
Draft

Fork anchored region from @ni/fast-foundation#3025
m-akinc wants to merge 3 commits into
mainfrom
users/makinc/forked-anchored-region

Conversation

@m-akinc

@m-akinc m-akinc commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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-foundation package, we want to fork the implementation into nimble-components first, then add the new API there.

👩‍💻 Implementation

  • Forked main impl file (index.ts), intersection-service.ts, and (meager) unit tests
    • Significantly refactored original updateLayout() 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.

@m-akinc
m-akinc marked this pull request as ready for review July 31, 2026 23:02
@m-akinc
m-akinc requested review from jattasNI and rajsite as code owners July 31, 2026 23:02
@rajsite

rajsite commented Jul 31, 2026

Copy link
Copy Markdown
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
rajsite marked this pull request as draft July 31, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants