feat(client): shared tabOrder property for keyboard Tab navigation across standard widgets#41895
Open
sebastianiv21 wants to merge 6 commits into
Open
feat(client): shared tabOrder property for keyboard Tab navigation across standard widgets#41895sebastianiv21 wants to merge 6 commits into
sebastianiv21 wants to merge 6 commits into
Conversation
Adds a single platform-level "tabOrder" property exposed in a shared Accessibility property-pane section for all standard non-Anvil widgets (injected centrally in WidgetFactory, no per-widget config changes). - New TAB_ORDER_INPUT control persists only valid non-negative integers as numbers and removes the property from the DSL when cleared, so blank always means Auto and no placeholder strings are persisted - PositionedContainer renders a sanitized data-tab-order attribute only for valid explicit values (0 is valid); no native tabIndex is used - Fixed-layout tabbing (useWidgetFocus) sorts widgets with explicit tabOrder first (ascending, duplicates tie-break by position), then Auto/invalid widgets in the existing position order; Shift+Tab reverses the same sequence. When no widget in the scope has a valid tabOrder, the previous position-based behavior is preserved exactly - Anvil-only (WDS_*, SECTION/ZONE) and internal widgets (CANVAS, SKELETON, TABS_MIGRATOR) are excluded; auto layout is intentionally unchanged since useWidgetFocus opts out of it - No DSL migration and no page version bump; existing apps keep their current Tab behavior unless tabOrder is explicitly set Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
WalkthroughAdds explicit tab order support for widgets. A new ChangesExplicit Widget Tab Order
Sequence Diagram(s)sequenceDiagram
participant User
participant TabOrderControl
participant sanitizeTabOrder
participant DSL as Widget DSL
participant PositionedContainer
participant sortTabbableWidgets
rect rgba(59, 130, 246, 0.5)
note over User, DSL: Property Panel: setting explicit tab order
User->>TabOrderControl: types "3"
TabOrderControl->>sanitizeTabOrder: "3"
sanitizeTabOrder-->>TabOrderControl: 3
TabOrderControl->>DSL: onPropertyChange("tabOrder", 3)
end
rect rgba(16, 185, 129, 0.5)
note over DSL, PositionedContainer: Render: emitting data-tab-order
DSL-->>PositionedContainer: tabOrder = 3
PositionedContainer->>sanitizeTabOrder: 3
sanitizeTabOrder-->>PositionedContainer: 3
PositionedContainer-->>User: data-tab-order="3" on DOM element
end
rect rgba(245, 158, 11, 0.5)
note over User, sortTabbableWidgets: Keyboard Tab navigation
User->>sortTabbableWidgets: Tab keypress, tabbable widgets
sortTabbableWidgets->>sanitizeTabOrder: reads data-tab-order per widget
sanitizeTabOrder-->>sortTabbableWidgets: explicit order or undefined
sortTabbableWidgets-->>User: next focused widget (explicit order first)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/27449717446. |
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. You can view the dependency diff in the run log. Look for the check-cyclic-dependencies job in the run. |
|
Deploy-Preview-URL: https://ce-41895.dp.appsmith.com |
Follow-up to the shared tabOrder feature (016eec8) after a "doesn't work" report. Investigation showed the feature is actually functional; the real gaps were clarity and missing end-to-end coverage. Investigation (verified live via Playwright on a deployed app): - Widget-level tab order WORKS at runtime for focusable widgets on the same canvas. A 3-widget test (orders top=1, mid=3, bottom=2) produced the keyboard focus cycle top -> bottom -> mid (ascending by tabOrder, not visual position), and data-tab-order propagated correctly to the deployed DOM. - The original "doesn't work" was a non-discriminating test: a 2-widget cycle ping-pongs identically whether ordered by position or tabOrder, so it cannot tell the two apart. Three widgets are required to distinguish them. - The genuine limitation is that fields INSIDE a form (JSON Form) cannot be individually ordered, because form fields are not standalone widgets and the control is injected per widget. This is by design: per-field tab numbering is the positive-tabindex accessibility anti-pattern. Within a form, native DOM order is used. Changes: - tabOrderPropertyConfig.ts: expand the help text to disclose that ordering is relative to widgets in the same container, and that fields inside a form follow the form's own order. Strings kept inline to match property-pane convention (sectionName/helpText are inline literals across the codebase). - tabbable.test.ts: add a discriminating 3-widget regression test asserting the tab sequence follows explicit order rather than position (forward and Shift+Tab). Asserts at the getTabbableDescendants level to avoid the pre-existing jsdom FOCUS_SELECTOR limitation that breaks handleTab-based tests. No feature flag added: the change is additive and inert by default (when no widget sets tabOrder, traversal falls back to the existing position-based path unchanged), so existing applications are unaffected. Verification: new test passes (suite 23 passed / 3 pre-existing jsdom failures); ESLint clean on changed files; check-types passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 1, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. You can view the dependency diff in the run log. Look for the check-cyclic-dependencies job in the run. |
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/27568604573. |
|
Deploy-Preview-URL: https://ce-41895.dp.appsmith.com |
The standalone WidgetProvider/factory/tabOrderPropertyConfig.ts imported constants/PropertyControlConstants and was imported by the WidgetFactory, closing a new import cycle that the ci-client-cyclic-deps-check (dpdm) gate flagged as +1 over the base branch. Move the shared tabOrder property-pane helpers into the existing WidgetProvider/factory/helpers.ts, which already imports every module they need (PropertyControlConstants, WidgetValidation, ./types, widgets/wds/constants) and is already part of these factory cycles. This adds no new import edge, so the dpdm circular-dependency count returns to the base 2134 with zero tabOrder-related cycles. - Delete tabOrderPropertyConfig.ts; functions/constants now live in helpers.ts - factory/index.tsx imports addTabOrderToPropertyPaneConfig from ./helpers - Rename the config test to helpers.tabOrder.test.ts, importing from ./helpers No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/27572072253. |
|
Deploy-Preview-URL: https://ce-41895.dp.appsmith.com |
- helpers.tabOrder.test.ts: assert the current help-text wording - PositionedContainer.test.tsx: add connect() to the react-redux mock so redux-form (pulled in via reflow selectors) loads - tabbable.test.ts: drop the two composite-widget handleTab tests; they hit querySelectorAll(FOCUS_SELECTOR) whose :is(...) syntax jsdom cannot parse, and exercise unchanged legacy routing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With the canvas native module available, the full suite runs and exposed
three jsdom/test-harness issues (not feature bugs):
- PositionedContainer.test.tsx: render the forwardRef default export instead
of the raw named function, which received frozen legacy context as its ref
arg ("object is not extensible")
- helpers.tabOrder.test.ts: stop calling getWidgetPropertyPaneConfig for every
registered widget with empty props (some widgets' dynamic-property
generators throw on {}). Prove blanket coverage via the pure
shouldExposeTabOrderProperty classifier across all types, and keep
representative end-to-end checks on static-pane widgets
- tabbable.test.ts: drop the handleTab describe; handleTab ends in
node.matches(FOCUS_SELECTOR), whose :is(...[tabindex='-1']) selector is
unparseable by jsdom/nwsapi. The ordering logic is fully covered via
getTabbableDescendants; handleTab is unchanged legacy plumbing
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/client/src/utils/hooks/useWidgetFocus/tabbable.test.ts (1)
1-2: ⚡ Quick winUse
TAB_ORDER_ATTRIBUTEin test fixtures instead of hardcodeddata-tab-order.This keeps test setup aligned with the shared contract and avoids silent drift if the attribute name changes.
Proposed refactor
import { getExplicitTabOrder, getNextTabbableDescendant, getTabbableDescendants, sortTabbableWidgets, sortWidgetsByPosition, } from "./tabbable"; +import { TAB_ORDER_ATTRIBUTE } from "utils/widgetTabOrder"; @@ if (tabOrder !== undefined) { - widget.setAttribute("data-tab-order", tabOrder); + widget.setAttribute(TAB_ORDER_ATTRIBUTE, tabOrder); } @@ - widget.setAttribute("data-tab-order", "0"); + widget.setAttribute(TAB_ORDER_ATTRIBUTE, "0"); @@ - widget.setAttribute("data-tab-order", "3"); + widget.setAttribute(TAB_ORDER_ATTRIBUTE, "3"); @@ - widget.setAttribute("data-tab-order", value); + widget.setAttribute(TAB_ORDER_ATTRIBUTE, value);Also applies to: 65-67, 98-103, 110-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/client/src/utils/hooks/useWidgetFocus/tabbable.test.ts` around lines 1 - 2, Replace all hardcoded `data-tab-order` attribute strings in test fixtures with the `TAB_ORDER_ATTRIBUTE` constant to keep test setup aligned with the shared contract. First, ensure `TAB_ORDER_ATTRIBUTE` is imported at the top of the file in app/client/src/utils/hooks/useWidgetFocus/tabbable.test.ts alongside `getExplicitTabOrder`. Then, at lines 65-67, 98-103, and 110-113 in the same file, replace each occurrence of the string `data-tab-order` with `TAB_ORDER_ATTRIBUTE` in test fixture HTML setup. This ensures the test will automatically reflect any future changes to the actual attribute name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/client/src/utils/hooks/useWidgetFocus/tabbable.test.ts`:
- Around line 1-2: Replace all hardcoded `data-tab-order` attribute strings in
test fixtures with the `TAB_ORDER_ATTRIBUTE` constant to keep test setup aligned
with the shared contract. First, ensure `TAB_ORDER_ATTRIBUTE` is imported at the
top of the file in app/client/src/utils/hooks/useWidgetFocus/tabbable.test.ts
alongside `getExplicitTabOrder`. Then, at lines 65-67, 98-103, and 110-113 in
the same file, replace each occurrence of the string `data-tab-order` with
`TAB_ORDER_ATTRIBUTE` in test fixture HTML setup. This ensures the test will
automatically reflect any future changes to the actual attribute name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a6369dec-3eb0-4870-8f15-440167711e31
📒 Files selected for processing (13)
app/client/src/WidgetProvider/factory/helpers.tabOrder.test.tsapp/client/src/WidgetProvider/factory/helpers.tsapp/client/src/WidgetProvider/factory/index.tsxapp/client/src/components/designSystems/appsmith/PositionedContainer.test.tsxapp/client/src/components/designSystems/appsmith/PositionedContainer.tsxapp/client/src/components/propertyControls/TabOrderControl.test.tsxapp/client/src/components/propertyControls/TabOrderControl.tsxapp/client/src/components/propertyControls/index.tsapp/client/src/layoutSystems/fixedlayout/common/PositionedComponentLayer.tsxapp/client/src/utils/hooks/useWidgetFocus/tabbable.test.tsapp/client/src/utils/hooks/useWidgetFocus/tabbable.tsapp/client/src/utils/widgetTabOrder.test.tsapp/client/src/utils/widgetTabOrder.ts
Replace hardcoded "data-tab-order" strings in the test fixtures with the shared TAB_ORDER_ATTRIBUTE constant so the setup tracks the real attribute name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Description
TL;DR: Adds a single platform-level
tabOrderproperty, exposed in a shared Accessibility property-pane section for every standard (non-Anvil) widget, letting builders optionally control keyboard Tab order in fixed layout. Existing apps keep their exact current Tab behavior unlesstabOrderis explicitly set.What changed
WidgetFactorycentrally appends one sharedAccessibility > Tab ordersection to both property-pane paths (legacygetPropertyPaneConfigand content config) viaWidgetProvider/factory/tabOrderPropertyConfig.ts. No per-widget config edits, no per-widget property names or validation rules. Anvil-only (WDS_*,SECTION_WIDGET,ZONE_WIDGET) and internal widgets (CANVAS_WIDGET,SKELETON_WIDGET,TABS_MIGRATOR_WIDGET) are excluded; widgets without a property pane stay untouched.TAB_ORDER_INPUTcontrol (TabOrderControl) reusing the ADSNumberInput: persists only valid non-negative integers as numbers, removes the property from the DSL when cleared (blank always means Auto), never persists""or invalid input. Not JS-convertible (isJSConvertible/isBindProperty/isTriggerProperty: false), validated asValidationTypes.NUMBER { min: 0, natural: true }.PositionedContainerrenders a sanitizeddata-tab-orderattribute only for valid explicit values (0is valid and earliest). No positive nativetabIndexis ever used, so non-focusable widgets stay non-focusable.useWidgetFocus/tabbable.ts) — newsortTabbableWidgets(): when no widget in the current scope has a valid explicit order, it delegates to the unchangedsortWidgetsByPosition(exact current behavior). Otherwise, explicitly ordered widgets come first ascending (duplicates tie-break by position), Auto/invalid widgets follow in existing position order, and Shift+Tab reverses the same sequence. Modal, container, JSONForm, CheckboxGroup, SwitchGroup and ButtonGroup behaviors are preserved.Compatibility
LATEST_PAGE_VERSIONbump, no widget default pollution —tabOrderonly exists in a DSL after a builder sets it.useWidgetFocusopts out of auto layout, sotabOrderis currently enforced only by the fixed-layout custom tabbing (FlexComponentandlayoutSystems/anvil/*untouched).Security notes
sanitizeTabOrder).Tests
utils/widgetTabOrder.test.ts— sanitizer value rules (0 valid; null/blank/decimals/negatives/NaN/Infinity/non-numeric → Auto).utils/hooks/useWidgetFocus/tabbable.test.ts— all-Auto preserves position order; explicit overrides position; 0 before 1; explicit widget above/left can be next; duplicate tie-break; blank/invalid → Auto; Shift+Tab; nested container scope; modal scope; composite widgets keep native internal tabbing.WidgetProvider/factory/tabOrderPropertyConfig.test.ts— unit tests plus an integration test over all registered widgets proving standard non-Anvil widgets expose exactly one identical shared property and Anvil/internal widgets expose none.components/propertyControls/TabOrderControl.test.tsx— persistence semantics incl. regression that clearing returns to Auto without persisting an invalid value.components/designSystems/appsmith/PositionedContainer.test.tsx—data-tab-orderrenders only for valid explicit values (including0), never a nativetabindex.Fixes #37947
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/27584120150
Commit: db268e7
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 16 Jun 2026 00:59:28 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Test Coverage