feat(core): add missing navigation/core widgets#82
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 413702de64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const zoneId = ctx.activeZoneId ?? findZoneForId(ctx.zones, focusedId); | ||
| if (zoneId === null) return null; | ||
|
|
||
| const tabsId = parseTabsContentZoneId(zoneId); | ||
| if (tabsId === null) return null; |
There was a problem hiding this comment.
The Escape path only works when activeZoneId itself is a tabs content zone id. If focus is inside a nested focusZone/focusTrap within the active tab panel, activeZoneId points to that nested zone, parseTabsContentZoneId(...) returns null, and the handler exits without moving focus back to the tab bar. This makes the documented Escape behavior fail for common tab content that contains its own focus-scoped widgets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7600d74.
What changed:
- Added zone ancestry metadata (
parentZoneId) in focus-zone collection/runtime metadata. - Updated
routeTabsKeyto walk parent zones from the active nested zone until it finds the tabs content zone, then return focus to the tab bar.
Regression coverage:
packages/core/src/widgets/__tests__/tabs.test.tsescape from nested zone inside tab content returns focus to tab bar
| const nextFocusedId = computeZoneMovement( | ||
| zone, | ||
| ctx.focusedId, | ||
| event.key === ZR_KEY_LEFT ? "left" : "right", |
There was a problem hiding this comment.
Left/Right navigation is driven by computeZoneMovement over focusable controls, then immediately presses whatever control receives focus. When ellipses are present, those ... items are non-focusable, so the next focusable can be a far page (or last), causing large page jumps rather than moving to page - 1 / page + 1. This violates the widget’s stated keyboard contract in large paginations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7600d74.
What changed:
- Updated
routePaginationKeyso Left/Right on page buttons routes by adjacent page intent (page - 1/page + 1) rather than by next focusable control. - If an adjacent page button is not currently visible due to ellipsis, routing now falls back to
prev/nextcontrols (still one-page movement) instead of jumping to far page controls.
Regression coverage:
packages/core/src/widgets/__tests__/pagination.test.tsright arrow on sparse page buttons uses next control instead of jumping pagesleft arrow on sparse page buttons uses prev control instead of jumping pages
1 participant
Summary
Adds the 4 missing essential widgets in core UI with stateful composition, rendering, and keyboard/focus integration:
ui.tabs()ui.accordion()ui.breadcrumb()ui.pagination()What changed
packages/core/src/widgets/andpackages/core/src/widgets/ui.tspackages/core/src/widgets/types.tspackages/core/src/runtime/router/packages/core/src/renderer/renderToDrawlist/widgets/navigation.tstabs.test.ts(26)accordion.test.ts(18)breadcrumb.test.ts(12)pagination.test.ts(15)Why
These components were missing from core and are required for standard app navigation/content disclosure patterns with accessible keyboard behavior and predictable focus management.
Validation
npm run buildnpm run typechecknode scripts/run-tests.mjsnpm run lint