Skip to content

feat: ButtonDropdown async loading and manual filtering - #5105

Merged
NathanZlion merged 28 commits into
mainfrom
dev-v3-natidere-button-dropdown-async-filtering
Oct 30, 2026
Merged

NathanZlion merged 28 commits into
mainfrom
dev-v3-natidere-button-dropdown-async-filtering

Conversation

@NathanZlion

@NathanZlion NathanZlion commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Description

Adds async loading and manual filtering to ButtonDropdown. These follow the same model Select and Multiselect already use.

Until now actions were required to be passed upfront synchronously. Changes in this PR enable Asynchronous Loading and Manual Filtering feature for Button Dropdown.

  • Async loading: the new onLoadItems event fires when the dropdown opens, when the user scrolls to the end, on Retry, and when a group is expanded. asyncLoadingProps sets the status (pending, loading, finished, error) and its texts.
  • Async groups: expandable groups can load their items when expanded. getExpandableItemsAsyncLoadingState reports each group's own status.
  • Manual filtering: with filteringType="manual", the component stops filtering on the client and passes the filter text to onLoadItems.

The status footer, live region and Retry are Select's existing shared code. useLoadItems is adapted from Select, with group loading added.

API proposal: K9PFTzwiUlNH · Bug bash: KytjqOibVjRy · Release sign-off: 9XKd8i0qDr2l

Related links, issue #, if available: n/a

How has this been tested?

  • Unit tests cover the main list, groups on desktop and mobile, Retry and focus, keyboard behaviour and manual filtering. There are no new integration tests.
  • The feature was bug bashed, and an A11Y sanity check has been requested.

To review, open the dev pages:

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.50249% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.75%. Comparing base (623c7d3) to head (644a8f6).

Files with missing lines Patch % Lines
src/button-dropdown/utils/use-button-dropdown.ts 96.96% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5105      +/-   ##
==========================================
+ Coverage   97.73%   97.75%   +0.01%     
==========================================
  Files         996      999       +3     
  Lines       32194    32382     +188     
  Branches    11886    12002     +116     
==========================================
+ Hits        31465    31654     +189     
- Misses        684      722      +38     
+ Partials       45        6      -39     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@NathanZlion
NathanZlion marked this pull request as ready for review October 5, 2026 14:57
@NathanZlion
NathanZlion requested a review from a team as a code owner October 5, 2026 14:57
@NathanZlion
NathanZlion requested review from SpyZzey and removed request for a team October 5, 2026 14:57
@SpyZzey
SpyZzey requested a balanced review from Copilot October 7, 2026 10:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Finished-status rows create invalid ARIA menu ownership on desktop and mobile.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
What changed in this PR

Adds asynchronous loading, expandable-group loading, and manual filtering to ButtonDropdown.

Changes:

  • Adds loading APIs, status footers, retry behavior, and i18n.
  • Supports manually filtered items and async expandable groups.
  • Adds unit tests, test utilities, snapshots, and demo pages.
File Description
src/​test-utils/​dom/​button-dropdown/​index.ts Adds scoped status and recovery selectors.
src/​i18n/​messages/​all.en.json Adds default retry and error labels.
src/​i18n/​messages-types.ts Types the new translations.
src/​button-dropdown/​utils/​use-load-items.ts Implements loading-event behavior.
src/​button-dropdown/​utils/​use-highlighted-menu.ts Tracks pending group highlights.
src/​button-dropdown/​utils/​use-button-dropdown.ts Integrates filtering, loading, retry, and group expansion.
src/​button-dropdown/​test-classes/​styles.scss Adds test-only status markers.
src/​button-dropdown/​status-footer.tsx Wraps shared status-footer interactions.
src/​button-dropdown/​items-list.tsx Passes async state into groups.
src/​button-dropdown/​internal.tsx Integrates the feature into rendering and behavior.
src/​button-dropdown/​internal-interfaces.ts Extends internal async-loading props.
src/​button-dropdown/​interfaces.ts Defines the new public API.
src/​button-dropdown/​index.tsx Forwards the public props.
src/​button-dropdown/​category-elements/​use-expandable-group-status.ts Derives per-group status UI.
src/​button-dropdown/​category-elements/​mobile-expandable-category-element.tsx Renders mobile group statuses.
src/​button-dropdown/​category-elements/​expandable-category-element.tsx Renders desktop group statuses.
src/​button-dropdown/​__tests__/​use-load-items.test.tsx Tests loading-event semantics.
src/​button-dropdown/​__tests__/​button-dropdown-async-loading.test.tsx Tests desktop async behavior.
src/​button-dropdown/​__tests__/​button-dropdown-async-loading-mobile.test.tsx Tests mobile async groups.
src/​__tests__/​snapshot-tests/​__snapshots__/​test-utils-selectors.test.tsx.snap Updates selector snapshots.
src/​__tests__/​snapshot-tests/​__snapshots__/​documenter.test.ts.snap Updates generated API snapshots.
pages/​button-dropdown/​manual-filtering.page.tsx Adds manual-filtering scenarios.
pages/​button-dropdown/​button-dropdown.async.example.page.tsx Adds a full async example.
pages/​button-dropdown/​async-loading.page.tsx Adds configurable async-loading scenarios.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/button-dropdown/internal.tsx Resolved
@NathanZlion
NathanZlion requested a review from pan-kot October 7, 2026 21:52
@SpyZzey

SpyZzey commented Oct 8, 2026

Copy link
Copy Markdown
Member

The behavior of the pending state is inconsistent between groups and non-groups.
Screenshot 2026-10-08 at 10 45 24
Screenshot 2026-10-08 at 10 45 36

Comment thread src/i18n/messages/all.en.json Resolved
- Render DropdownStatus directly (no DropdownFooter border) when
  items are empty and status is sticky (loading/error state)
- Apply fix to both main dropdown (internal.tsx) and expandable
  group sub-dropdown (expandable-category-element.tsx)
- Remove dead code: fireGroupLoadItems was exported but never called
- Add use-load-items.test.tsx covering handleLoadMore and
  handleRecoveryClick branches
- Improve test coverage for DropdownStatus direct-render path
…recovery

- Share the expandable group status logic between the desktop fly-out and the
  mobile inline group so both render loading, error, empty, and finished states
- Route group recovery through the root so focus returns to the filter input in
  filtering mode; group events never carry the root filtering text
- Pass the real async status to the options list so a short first page loads
  the next one, as in Select
- Treat a group status of pending as finished: groups load as a single page
- Test utils: expandedGroupDropdown also resolves inline mobile groups; drop the
  duplicate stylesheet import
Reference getExpandableItemsAsyncLoadingState instead of a non-existent prop,
describe when onLoadItems fires, document that the text properties receive the
group id, and use items instead of options throughout.
Status, item presets, filtering type, delay, and expandToViewport are now
AppContext URL params so each state is linkable; fetched results stay local.
Descriptions now match the actual 5 s timeouts.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Virtual focus can reference missing or duplicate DOM IDs, causing incorrect accessibility behavior.

1 open issue
2 previously missed issues

In code that hasn't changed since last review

Medium severity Menu item IDs collide with status and recovery IDs

src/​button-dropdown/​category-elements/​use-expandable-group-status.ts:32

These IDs collide with menu-item IDs (${menuId}-${item.id}). For example, group g1 and a distinct action g1-recovery get the same DOM ID when Retry appears; g1-status similarly collides with the status. In filtering mode, aria-activedescendant can then resolve to the wrong element. Prefix the status and recovery IDs before menuId to separate them from item IDs.

Medium severity Extract shared request and pagination logic into a hook

src/​button-dropdown/​utils/​use-load-items.ts:17

The reuse-before-adding convention calls for one shared implementation. This duplicates Select's request tracking, pagination, and retry logic (src/select/utils/use-load-items.ts:16–43), with an additional missing-handler guard here. Extract the common root behavior into an internal hook and compose the group-specific events around it so loading fixes do not need separate implementations.

4 resolved since last review

Review effort: Balanced

Comment thread src/button-dropdown/internal.tsx Resolved
Comment on lines +265 to +272
fetchGroupItems(expandedGroupId)
.then(items => {
setGroupItems(prev => ({ ...prev, [expandedGroupId]: items }));
setGroupStatuses(prev => ({ ...prev, [expandedGroupId]: 'finished' }));
})
.catch(() => {
setGroupStatuses(prev => ({ ...prev, [expandedGroupId]: 'error' }));
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is technically a race condition here with fetchGroupItems. When a user opens and closes the group multiple times, it will create multiple promises, which all resolve independent form each other, populating the setGroupItems and Status. This way, the 5s delay is not really visible.

Though this might be fine for a test page. You can decide whether you want to fix this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have solved this in the other files by using requestId

empty: (gid?: string) => `No items in ${gid ?? 'group'}.`,
};

export default function ButtonDropdownAsyncLoadingPage() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is huge. Can we move the switch out to clean up the code?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be a follow-up

};

// Long enough to inspect the loading state and for integration tests to assert on it.
const FETCH_DELAY_MS = 5000;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make it so this is can be changed in the settings? It's quite annoying when testing manually

// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import { fireNonCancelableEvent } from '../../internal/events';
import { useLoadItems as useBaseLoadItems } from '../../select/utils/use-load-items';

@SpyZzey SpyZzey Oct 29, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ: as a follow up, should we refactor this to not sit in select/?

Comment on lines +11 to +12
options: ReadonlyArray<unknown>;
statusType: DropdownStatusProps.StatusType | undefined;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: for a follow-up: Can't we remove options completely? It only checks whether it's an empty array. we could have a property "iconCount" or isEmpty instead.

@NathanZlion
NathanZlion added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit cc35fe2 Oct 30, 2026
69 checks passed
@NathanZlion
NathanZlion deleted the dev-v3-natidere-button-dropdown-async-filtering branch October 30, 2026 06:03

This branch was successfully deployed

3 active deployments
dev-pages-react18 — 644a8f62 Deployed Oct 21, 2026 by NathanZlion via deploy (React 18) / deploy #2907
dev-pages-react16 — 644a8f62 Deployed Oct 21, 2026 by NathanZlion via deploy (React 16) / deploy #2907
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.

3 participants