UI: Count filtered items in FilterPanel#34607
Draft
Sidnioulz wants to merge 7 commits into
Draft
Conversation
Contributor
📝 WalkthroughWalkthroughThe changes extend the filter sidebar to display visible item counts alongside total counts. A Changes
Sequence DiagramsequenceDiagram
participant State as Manager State
participant Filter as Filter Component
participant FilterPanel as FilterPanel Component
participant Compute as Visible Count<br/>Computation
participant Display as FilterPanelLink<br/>(UI Render)
State->>Filter: Provide filteredIndex
Filter->>FilterPanel: Pass filteredIndex prop
FilterPanel->>Compute: Compute visible counts<br/>(filteredTagCounts,<br/>filteredBuiltInCounts,<br/>filteredStatusCounts)
Compute-->>FilterPanel: Return count objects
FilterPanel->>FilterPanel: Create FilterItems with<br/>visibleCount + total count
FilterPanel->>Display: Pass FilterItems to links
Display->>Display: Format count string<br/>(visibleCount / totalCount)
Display-->>Display: Render in UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/core/src/manager/components/sidebar/FilterPanel.tsx (1)
95-101: Type assertion is fragile if filter function signatures evolve.The comment explains the rationale, but the
as unknown asdouble-cast bypasses type safety. IfgetFilterFunctionreturns functions expecting additional properties in the future, this would silently break.Consider adding a type guard or narrowing the filter function type to explicitly accept
API_LeafEntry:Alternative approach
- counts[entry.id] = filteredEntries.filter((e) => - filterFn?.(e as unknown as Parameters<typeof filterFn>[0]) - ).length; + counts[entry.id] = filteredEntries.filter((e) => { + // API_LeafEntry has the properties filter functions need: type, tags, subtype + return filterFn?.({ type: e.type, tags: e.tags, subtype: 'subtype' in e ? e.subtype : undefined } as Parameters<typeof filterFn>[0]); + }).length;Or define a minimal type that both
API_LeafEntryand the filter function parameter share.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/manager/components/sidebar/FilterPanel.tsx` around lines 95 - 101, Replace the fragile "as unknown as" double-cast by ensuring the filter function and the entries share a concrete, narrow type: introduce a minimal shared type/interface (e.g., FilterableEntry) that includes the properties used by filters (type, tags, subtype), update getFilterFunction's returned function signature to accept FilterableEntry (or wrap its result in a type-guarding adapter), and then call filterFn with entries typed as FilterableEntry; update references to builtInEntries, getFilterFunction, filteredEntries and counts accordingly so no unsafe casting is needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/manager/components/sidebar/FilterPanel.tsx`:
- Around line 72-86: The computed filtered counts (filteredTagCounts,
filteredBuiltInCounts, filteredStatusCounts) return empty when filteredIndex is
undefined, causing visible counts to show 0; update each useMemo to fall back to
the full/unfiltered index when filteredIndex is undefined (e.g., use
filteredIndex ?? index or filteredIndex ?? unfilteredIndex depending on the
component state variable that holds all items) so the reduce runs over the full
dataset when no external filter is applied; ensure you reference the same
tag/status/built-in logic (BUILT_IN_TAGS, entry.type checks, entry.tags) but
iterate the unfiltered collection in that fallback case.
---
Nitpick comments:
In `@code/core/src/manager/components/sidebar/FilterPanel.tsx`:
- Around line 95-101: Replace the fragile "as unknown as" double-cast by
ensuring the filter function and the entries share a concrete, narrow type:
introduce a minimal shared type/interface (e.g., FilterableEntry) that includes
the properties used by filters (type, tags, subtype), update getFilterFunction's
returned function signature to accept FilterableEntry (or wrap its result in a
type-guarding adapter), and then call filterFn with entries typed as
FilterableEntry; update references to builtInEntries, getFilterFunction,
filteredEntries and counts accordingly so no unsafe casting is needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e4926d8-bdbb-47da-92a7-12e06c2c2cb1
📒 Files selected for processing (5)
code/core/src/manager/components/sidebar/Filter.tsxcode/core/src/manager/components/sidebar/FilterPanel.stories.tsxcode/core/src/manager/components/sidebar/FilterPanel.tsxcode/core/src/manager/components/sidebar/FilterPanel.utils.tscode/core/src/manager/components/sidebar/FilterPanelLink.tsx
Comment on lines
+72
to
+86
| // Compute filtered counts for custom tags | ||
| const filteredTagCounts = useMemo(() => { | ||
| return Object.values(filteredIndex || {}).reduce<{ | ||
| [key: Tag]: number; | ||
| }>((acc, entry) => { | ||
| if (['story', 'docs'].includes(entry.type)) { | ||
| entry.tags?.forEach((tag: Tag) => { | ||
| if (!BUILT_IN_TAGS.has(tag)) { | ||
| acc[tag] = (acc[tag] || 0) + 1; | ||
| } | ||
| }); | ||
| } | ||
| return acc; | ||
| }, {}); | ||
| }, [filteredIndex]); |
Contributor
There was a problem hiding this comment.
Visible count is 0 when no filtering is applied — likely incorrect UX.
When filteredIndex is undefined (no external filtering applied), these computations return empty objects, resulting in visibleCount = 0 for all items via the ?? 0 fallbacks (lines 187, 196, 204). The UI would display "0 / 10" instead of "10 / 10" when there's no filtering.
Consider falling back to the total count when filteredIndex is undefined:
Proposed fix
const filteredTagCounts = useMemo(() => {
+ // When no external filtering is applied, visible equals total
+ if (!filteredIndex) {
+ return tagEntries.reduce<Record<Tag, number>>((acc, entry) => {
+ acc[entry.id] = entry.count;
+ return acc;
+ }, {});
+ }
return Object.values(filteredIndex || {}).reduce<{
[key: Tag]: number;
}>((acc, entry) => {Similar changes would be needed for filteredBuiltInCounts and filteredStatusCounts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/manager/components/sidebar/FilterPanel.tsx` around lines 72 -
86, The computed filtered counts (filteredTagCounts, filteredBuiltInCounts,
filteredStatusCounts) return empty when filteredIndex is undefined, causing
visible counts to show 0; update each useMemo to fall back to the
full/unfiltered index when filteredIndex is undefined (e.g., use filteredIndex
?? index or filteredIndex ?? unfilteredIndex depending on the component state
variable that holds all items) so the reduce runs over the full dataset when no
external filter is applied; ensure you reference the same tag/status/built-in
logic (BUILT_IN_TAGS, entry.type checks, entry.tags) but iterate the unfiltered
collection in that fallback case.
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 18 | 18 | 0 |
| Self size | 1.26 MB | 1.67 MB | 🚨 +407 KB 🚨 |
| Dependency size | 9.27 MB | 9.27 MB | 🎉 -735 B 🎉 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 72 | 50 | 🎉 -22 🎉 |
| Self size | 20.25 MB | 20.55 MB | 🚨 +297 KB 🚨 |
| Dependency size | 36.17 MB | 16.57 MB | 🎉 -19.61 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 662 KB | 650 KB | 🎉 -12 KB 🎉 |
| Dependency size | 61.37 MB | 61.34 MB | 🎉 -32 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 93 | 93 | 0 |
| Self size | 1.38 MB | 1.12 MB | 🎉 -266 KB 🎉 |
| Dependency size | 24.79 MB | 24.77 MB | 🎉 -27 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 122 | 122 | 0 |
| Self size | 30 KB | 30 KB | 🚨 +5 B 🚨 |
| Dependency size | 25.86 MB | 25.84 MB | 🎉 -26 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 36 KB | 36 KB | 🎉 -14 B 🎉 |
| Dependency size | 22.57 MB | 22.54 MB | 🎉 -26 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 23 KB | 23 KB | 🎉 -7 B 🎉 |
| Dependency size | 45.91 MB | 45.88 MB | 🎉 -32 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 184 | 🎉 -19 🎉 |
| Self size | 908 KB | 782 KB | 🎉 -126 KB 🎉 |
| Dependency size | 87.56 MB | 68.31 MB | 🎉 -19.25 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 177 | 🎉 -19 🎉 |
| Self size | 32 KB | 32 KB | 🎉 -34 B 🎉 |
| Dependency size | 86.04 MB | 66.83 MB | 🎉 -19.21 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 51 | 🎉 -22 🎉 |
| Self size | 1.08 MB | 1.04 MB | 🎉 -36 KB 🎉 |
| Dependency size | 56.43 MB | 37.12 MB | 🎉 -19.31 MB 🎉 |
| Bundle Size Analyzer | node | node |
@storybook/react-dom-shim
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 0 | 0 | 0 |
| Self size | 19 KB | 19 KB | 🎉 -276 B 🎉 |
| Dependency size | 1 KB | 791 B | 🎉 -309 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.47 MB | 1.44 MB | 🎉 -29 KB 🎉 |
| Dependency size | 13.30 MB | 13.30 MB | 🎉 -853 B 🎉 |
| Bundle Size Analyzer | Link | Link |
| </span> | ||
| </ActionList.Text> | ||
| {isExcluded ? <s>{count}</s> : <span>{count}</span>} | ||
| {isExcluded ? <s>{countString}</s> : <span>{countString}</span>} |
Contributor
There was a problem hiding this comment.
Screen readers announce 3 / 12 as "3 slash 12". Shouldn't we add an aria-label?:
const ariaLabel = isNarrowed ? `${visibleCount} of ${count} items` : `${count} items`;
<span aria-label={ariaLabel}>...</span>
Contributor
There was a problem hiding this comment.
Handled in 5bf2206. The old visible / total text is gone; the projected counts now use explicit aria-labels for both the row values and the panel summary so screen readers announce the count change naturally.
Screenshot:
Sidnioulz
commented
May 20, 2026
Contributor
Author
Sidnioulz
left a comment
Sidnioulz
left a comment
There was a problem hiding this comment.
@copilot completely rebuild this from scratch. Instead of showing visibleCount / totalCount, we'll show how many items are added or removed when including or excluding a tag.
For each filter item, count how many items would be added/removed by including/excluding it. Memoise that as it's costly to compute.
Then, instead of visibleCount / totalCount, we're gonna split the display of tag information in three places:
- At the top of the FilterPanel, we'll add a
visibleCount / totalCountfor all the entries in the index, e.g. 10/2000 if we include tagsfoowith 5 entries andbarwith 5 entries in an index with 2000 entries - On each filter list item, we'll show the amount of items shown for the current filters, e.g. in the example above,
fooandbarwould show 5 and the rest 0; all excluded tags would always show 0 - When hovering/focusing an item, we'll show the delta it would produce on the overall index, e.g.
+10,-290, etc. on the item being hovered, and we'll show the "new" visible count at the top of the panel if it is applied, e.g. if we add tagterthat has 15 entries (5 of which overlap withfoo), then it would show +10 on the entry, and 20/2000 on the top counter
You may propose a color or specific style to show the "temporary" counter, I don't have particular ideas. There will likely be adjustments but build that to let us get a feel of how it behaves. Add relevant stories.
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/88ef0542-1b73-4194-9dbb-79bdbeb2bf00 Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/88ef0542-1b73-4194-9dbb-79bdbeb2bf00 Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/88ef0542-1b73-4194-9dbb-79bdbeb2bf00 Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Agent-Logs-Url: https://github.com/storybookjs/storybook/sessions/88ef0542-1b73-4194-9dbb-79bdbeb2bf00 Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Contributor
Reworked in 988bc02 and polished in 5bf2206. The panel now shows the current overall visible/total count at the top, each row shows the count currently visible for that filter, and hover/focus previews show the include/exclude delta plus the projected top count if applied. I also added a focused story and utility coverage for the projection logic. Screenshot: Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…unt for all filters - Move the SummaryRow (Shown/If applied count) to sit on the right of the Select all / Clear filters row, using margin-inline-start: auto to push it to the end of the flex container - Fix visibleCount always returning 0 for non-included filters: remove the isIncluded guard so every filter always shows how many currently-visible items match it - Update the test expectation to match the corrected behaviour (ter has 5 visible items when foo+bar are active, since foo-1..5 carry both tags) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
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.
Closes #32413
TODO
We've agreed to rework this PR so that tooltips announce how many entries would be added/removed if a specific filter is applied as include/exclude.
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Tests