Skip to content

refactor(data-table): remove redundant toolbar effects - #440

Open
IzumiSy wants to merge 1 commit into
stack/data-table-filter-placement-and-column-ui-followupsfrom
datatable_toolbar_refactor
Open

refactor(data-table): remove redundant toolbar effects#440
IzumiSy wants to merge 1 commit into
stack/data-table-filter-placement-and-column-ui-followupsfrom
datatable_toolbar_refactor

Conversation

@IzumiSy

@IzumiSy IzumiSy commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Motivation

The data-table toolbar had two local useEffect hooks that were doing work already covered elsewhere: one only needed to react to the field-search input itself, and the other duplicated Base UI popover focus behavior. Removing them keeps the toolbar state flow more direct without changing the user-facing interaction.

Design Decision

Chosen approach

  • Move the add-filter field selection sync into the field-search input change handler, since that correction only matters when the query changes.
  • Remove the operator picker focus effect and rely on Base UI Popover.Popup's default initial focus behavior.
  • Leave an inline comment documenting the focus behavior, including the touch-specific case where the popup keeps focus to avoid opening the virtual keyboard.
  • Add a regression test that verifies the operator search input is focused when its popover opens.

Alternatives considered

  • Keep both effects. This works, but it leaves imperative synchronization in places where existing event flow and library behavior already cover the need.
  • Replace the operator picker effect with autoFocus. That also focuses the input, but it would bypass Base UI's built-in touch behavior.

Summary

  • remove the add-filter search-sync effect in favor of a search input callback
  • remove the operator picker focus effect and document the Base UI focus contract
  • add a toolbar regression test for operator search focus

@IzumiSy IzumiSy changed the title refactor(data-table): remove redundant toolbar effects fix(data-table): restore add-filter toolbar updates Aug 13, 2026
@IzumiSy IzumiSy changed the title fix(data-table): restore add-filter toolbar updates refactor(data-table): remove redundant toolbar effects Aug 13, 2026
@IzumiSy
IzumiSy changed the base branch from main to stack/data-table-filter-placement-and-column-ui-followups August 13, 2026 04:05
@IzumiSy
IzumiSy force-pushed the datatable_toolbar_refactor branch from 10f83ef to 9db0ff0 Compare August 13, 2026 04:39
@IzumiSy
IzumiSy marked this pull request as ready for review August 13, 2026 07:31
@IzumiSy
IzumiSy requested a review from a team as a code owner August 13, 2026 07:31
@IzumiSy

IzumiSy commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

API Design Review completed successfully!

API Design Review complete for PR #440 (refactor(data-table): remove redundant toolbar effects). No High or Medium issues found. The handleFieldQueryChange refactor is equivalent to the removed useEffect, the Base UI focus contract is documented inline, and a regression test was added. Verdict: Approve — no unresolved issues.

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.

1 participant