UI: Sync FilterBar pills with URL values#67082
Conversation
suyua9
requested review from
bbovenzi,
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
suyua9
force-pushed
the
fix/filterbar-sync-initial-values
branch
from
a4ca2d4 to
ab083c4
Compare
|
For UI-related contributions, attaching before/after screenshots to the PR description would help reviewers a lot. Could you update the description with screenshots when you have a chance? |
potiuk
added
the
ready for maintainer review
pierrejeambrun
added
backport-to-v3-2-test
There was a problem hiding this comment.
Thanks for the fix! Direction looks good and the test covers the new behavior. One small nit below, nothing blocking.
For UI-related contributions, attaching before/after screenshots to the PR description would help reviewers a lot.
Could you update the description with screenshots when you have a chance?
That. Or give more context / reproduction step about what this is actually fixing. I tried manually editing the state filter in the URL and refresh the page, but that's already working on main, so I'm not sure what this is solving yet.
| }; | ||
|
|
||
| const queryPillText = (value: string) => | ||
| screen.queryAllByText((_, element) => element?.textContent === `State: ${value}`).at(-1); |
There was a problem hiding this comment.
Nit: the getAllByText(...).at(-1) selector is a little fragile since it depends on the order of text matches in the rendered DOM. Could we add a data-testid on the pill in FilterPill.tsx (something like data-testid={\filter-pill-${filter.config.key}}) and use screen.getByTestId in the test? Makes the test more readable too.
4 participants
Fix FilterBar reconciliation so existing pills update their visible value when external URL state rewrites
initialValues.The existing effect already added new pills and removed cleared values. This also replaces the value for an already-rendered pill, which avoids stale UI when URL-driven table filters change externally.
Tests:
./node_modules/.bin/eslint --quiet src/components/FilterBar/FilterBar.tsx src/components/FilterBar/FilterBar.test.tsx./node_modules/.bin/vitest run src/components/FilterBar/FilterBar.test.tsxNo newsfragment added; this is a small UI bug fix.
Was generative AI tooling used to co-author this PR?
Generated-by: OpenAI Codex following the guidelines