Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Fix controls and parameters on tag-filtered stories #30038

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

shilman
Copy link
Member

@shilman shilman commented Dec 12, 2024

Closes #29827

What I did

Refactor the manager to include a full stories hash and a filtered stories hash. Show the filtered hash in the sidebar but use the full stories hash for storing parameters etc.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  1. Start a sandbox
  2. Select a story that shows controls
  3. Use tags filtering to filter out that story from the sidebar
  4. Verify that the controls still render successfully

🦋 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/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.7 MB 77.7 MB 0 B 0.65 0%
initSize 133 MB 133 MB -17 kB 0.33 0%
diffSize 55.3 MB 55.3 MB -17 kB 0.32 0%
buildSize 6.87 MB 6.87 MB 1.63 kB 5.31 0%
buildSbAddonsSize 1.51 MB 1.51 MB 843 B 2.38 0.1%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.86 MB 1.86 MB 734 B 11.54 0%
buildSbPreviewSize 0 B 0 B 0 B - -
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.57 MB 3.57 MB 1.58 kB 5.33 0%
buildPreviewSize 3.3 MB 3.3 MB 49 B 0.8 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 7.5s 7.5s 70ms -0.84 0.9%
generateTime 20.3s 21.3s 937ms -0.3 4.4%
initTime 13.1s 14s 903ms -0.44 6.4%
buildTime 9.5s 7.8s -1s -680ms -1.83 🔰-21.4%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 5s 4.3s -614ms -1.19 -14%
devManagerResponsive 3.7s 3.3s -387ms -1.04 -11.5%
devManagerHeaderVisible 549ms 521ms -28ms -0.69 -5.4%
devManagerIndexVisible 626ms 552ms -74ms -1.09 -13.4%
devStoryVisibleUncached 1.8s 1.5s -308ms -1.2 -20.1%
devStoryVisible 624ms 553ms -71ms -0.98 -12.8%
devAutodocsVisible 507ms 520ms 13ms 0.1 2.5%
devMDXVisible 483ms 446ms -37ms -0.78 -8.3%
buildManagerHeaderVisible 585ms 511ms -74ms -0.78 -14.5%
buildManagerIndexVisible 692ms 603ms -89ms -0.78 -14.8%
buildStoryVisible 534ms 475ms -59ms -0.68 -12.4%
buildAutodocsVisible 410ms 431ms 21ms -0.35 4.9%
buildMDXVisible 415ms 390ms -25ms -0.75 -6.4%

Greptile Summary

This PR refactors Storybook's manager to maintain both full and filtered story indexes, ensuring Controls panel functionality remains intact when stories are filtered by tags in the sidebar.

  • Added filteredIndex field to API_LoadedRefData interface in code/core/src/types/modules/api.ts
  • Modified setIndex in code/core/src/manager-api/modules/stories.ts to maintain both filtered and unfiltered story hashes
  • Updated sidebar components to use filteredIndex for display while preserving full story data for Controls
  • Added test coverage in code/core/src/manager-api/tests/stories.test.ts for filtered index functionality
  • Modified refs module to handle both indexes in code/core/src/manager-api/modules/refs.ts

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

9 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

},
// FIXME: is there a bug where filtered stories get added back in on updateStory???
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: This FIXME comment identifies a real issue - filtered stories could get re-added during updates. Consider adding a check to prevent this.

code/core/src/manager-api/modules/stories.ts Outdated Show resolved Hide resolved
code/core/src/manager-api/modules/stories.ts Show resolved Hide resolved
Copy link

nx-cloud bot commented Dec 12, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 6dc2790. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@shilman shilman force-pushed the shilman/fix-tag-filtered-controls branch from 85af220 to bea7aec Compare December 12, 2024 14:06
@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Dec 12, 2024

Package Benchmarks

Commit: 6dc2790, ran on 13 December 2024 at 12:04:48 UTC

The following packages have significant changes to their size or dependencies:

@storybook/addon-a11y

Before After Difference
Dependency count 59 59 0
Self size 411 KB 45 KB 🎉 -366 KB 🎉
Dependency size 13.46 MB 13.46 MB 0 B
Bundle Size Analyzer Link Link

@storybook/addon-onboarding

Before After Difference
Dependency count 0 2 🚨 +2 🚨
Self size 235 KB 216 KB 🎉 -19 KB 🎉
Dependency size 670 B 235 KB 🚨 +235 KB 🚨
Bundle Size Analyzer Link Link

@storybook/experimental-nextjs-vite

Before After Difference
Dependency count 153 87 🎉 -66 🎉
Self size 231 KB 231 KB 🎉 -16 B 🎉
Dependency size 44.67 MB 31.57 MB 🎉 -13.10 MB 🎉
Bundle Size Analyzer Link Link

@shilman shilman force-pushed the shilman/fix-tag-filtered-controls branch from bea7aec to 56f5220 Compare December 13, 2024 11:24
@shilman shilman force-pushed the shilman/fix-tag-filtered-controls branch from 56f5220 to 6dc2790 Compare December 13, 2024 11:54
@shilman shilman merged commit d3cce51 into next Dec 13, 2024
58 checks passed
@shilman shilman deleted the shilman/fix-tag-filtered-controls branch December 13, 2024 13:15
@github-actions github-actions bot mentioned this pull request Dec 13, 2024
17 tasks
@shilman shilman added the needs qa Indicates that this needs manual QA during the upcoming minor/major release label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ci:normal needs qa Indicates that this needs manual QA during the upcoming minor/major release tags
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Stories without the 'dev' tags (or filtered stories in general) have empty Controls panel
1 participant