-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Sidebar: Add option for weighted story sorting #28722
base: next
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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, 1 comment(s)
Edit PR Review Bot Settings
if (!weightA) { | ||
weightA = 0; | ||
} | ||
if (!weightB) { | ||
weightB = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic: Potential issue: Defaulting weights to 0 might cause unintended sorting behavior if weights are not explicitly set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only used if one story in the comparison has a weight and the other doesn't. Un-weighted stories shouldn't affect the sorting one way or the other, so setting their weight to 0 ensures that the weighted story takes precedence. "Lighter" (negative weights) stories rise to the top, and "heavier" (positive weights) sink to the bottom.
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 7063e90. 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 targetSent with 💌 from NxCloud. |
What are the next steps for a PR like this? Is there a recurring meeting where the maintainers discuss potential new features, or something like that? Obviously I am eager for this to be added, but I realize there is probably a process beyond "lgtm.. merge". |
@chimericdream Thanks for your contribution and also for your patience. I got assigned this one while I was on vacation, and am just looking at it now. We have a triage meeting every Monday at 1PM GMT in Storybook's Discord #watercooler channel. I'll bring it up with the team next week and discuss and post back here. If that time works for you, you're also very welcome to attend. https://discord.gg/storybook |
@shilman no worries! I'll see if I can attend the meeting next week, but either way I'm looking forward to hearing back. |
@shilman how did the meeting go? Sorry I wasn't able to attend. |
Closes #27521
What I did
This PR adds a new option to the
storySort
config calledweights
. This is a simple map of page titles to numbers. Its purpose is to provide an alternate syntax for situations where pages with a certain title (e.g. "Introduction" or "Appendix") should be sorted to the top or bottom of the story list, regardless of how deeply nested they are.For example, the following is a simplified version of the
storySort
config for my team's monorepo:With this PR, we should be able to replace that config with the following:
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
I tested this manually by building the core package and using it in a local Storybook application (the one mentioned in the discussion above).
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
Make 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/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>
Greptile Summary
This PR introduces a new
weights
option to thestorySort
configuration, allowing for more flexible and intuitive story sorting based on numerical weights assigned to specific story titles.code/core/src/preview-api/modules/store/storySort.ts
to incorporate weighted sorting logic.code/core/src/csf-tools/getStorySortParameter.test.ts
andcode/core/src/preview-api/modules/store/storySort.test.ts
to validate the newweights
option.code/core/src/types/modules/addons.ts
to include the newweights
property in theAddon_StorySortObjectParameter
interface.docs/writing-stories/naming-components-and-hierarchy.mdx
and added new snippets to demonstrate the usage of theweights
option.eval
incode/core/src/csf-tools/getStorySortParameter.ts
could pose security risks if not properly sanitized.