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

Sidebar: Add option for weighted story sorting #28722

Open
wants to merge 20 commits into
base: next
Choose a base branch
from

Conversation

chimericdream
Copy link

@chimericdream chimericdream commented Jul 27, 2024

Closes #27521

What I did

This PR adds a new option to the storySort config called weights. 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:

storySort: {
    order: [
        'Introduction',
        'Team Docs',
        [
            'Onboarding',
            ['Introduction', ['*', ['Introduction']]],
            'Monorepo',
            ['Introduction', ['*', ['Introduction']]],
        ],
        'Apps',
        [
            'App1',
            ['Introduction', ['*', ['Introduction']]],
            'App2',
            ['Introduction', ['*', ['Introduction']]],
        ],
        'Services',
        [
            'API 1',
            ['Introduction', ['*', ['Introduction']]],
            'API 2',
            ['Introduction', ['*', ['Introduction']]],
        ],
        'Libraries',
        ['Introduction', ['*', ['Introduction']]],
        'Design System',
        ['Introduction', ['*', ['Introduction']]],
        'Component Library',
        ['Introduction', ['*', ['Introduction']]],
    ],
},

With this PR, we should be able to replace that config with the following:

storySort: {
    weights: { 'Introduction': -1 },
    order: [
        'Team Docs',
        [ 'Onboarding', 'Monorepo' ],
        'Apps',
        [ 'App1', 'App2' ],
        'Services',
        [ 'API 1', 'API 2' ],
        'Libraries',
        'Design System',
        'Component Library',
    ],
},

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

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

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/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>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.7 MB 77.7 MB 452 B 0.73 0%
initSize 152 MB 152 MB 99.5 kB -0.95 0.1%
diffSize 73.9 MB 74 MB 99 kB -0.95 0.1%
buildSize 6.77 MB 6.77 MB 310 B -1.14 0%
buildSbAddonsSize 1.5 MB 1.5 MB 200 B -1.48 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.83 MB 1.83 MB 0 B -0.87 0%
buildSbPreviewSize 270 kB 270 kB 156 B -1.78 0.1%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.8 MB 3.8 MB 356 B -1.19 0%
buildPreviewSize 2.97 MB 2.97 MB -46 B -0.5 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.6s 22.7s 15s 1.39 🔺66.1%
generateTime 25.4s 24.2s -1s -153ms 1.38 -4.7%
initTime 18.3s 18s -329ms 1.07 -1.8%
buildTime 11.1s 8.3s -2s -778ms -1.52 🔰-33.2%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6.8s 7.7s 949ms 0.48 12.2%
devManagerResponsive 4.4s 5s 538ms 0.51 10.7%
devManagerHeaderVisible 596ms 737ms 141ms 0.54 19.1%
devManagerIndexVisible 630ms 755ms 125ms 0.39 16.6%
devStoryVisibleUncached 782ms 1.4s 636ms 0.58 44.9%
devStoryVisible 629ms 746ms 117ms 0.3 15.7%
devAutodocsVisible 557ms 639ms 82ms 0.65 12.8%
devMDXVisible 523ms 673ms 150ms 0.79 22.3%
buildManagerHeaderVisible 531ms 642ms 111ms 0.28 17.3%
buildManagerIndexVisible 533ms 721ms 188ms 0.61 26.1%
buildStoryVisible 566ms 722ms 156ms 0.38 21.6%
buildAutodocsVisible 456ms 527ms 71ms -0.23 13.5%
buildMDXVisible 648ms 650ms 2ms 0.65 0.3%

Greptile Summary

This PR introduces a new weights option to the storySort configuration, allowing for more flexible and intuitive story sorting based on numerical weights assigned to specific story titles.

  • Enhanced Sorting Logic: Modified code/core/src/preview-api/modules/store/storySort.ts to incorporate weighted sorting logic.
  • New Test Cases: Added tests in code/core/src/csf-tools/getStorySortParameter.test.ts and code/core/src/preview-api/modules/store/storySort.test.ts to validate the new weights option.
  • Type Definitions: Updated code/core/src/types/modules/addons.ts to include the new weights property in the Addon_StorySortObjectParameter interface.
  • Documentation Updates: Modified docs/writing-stories/naming-components-and-hierarchy.mdx and added new snippets to demonstrate the usage of the weights option.
  • Security Consideration: The use of eval in code/core/src/csf-tools/getStorySortParameter.ts could pose security risks if not properly sanitized.

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, 1 comment(s)
Edit PR Review Bot Settings

Comment on lines 51 to 55
if (!weightA) {
weightA = 0;
}
if (!weightB) {
weightB = 0;
Copy link
Contributor

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.

Copy link
Author

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.

Copy link

nx-cloud bot commented Jul 27, 2024

☁️ Nx Cloud Report

CI 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 target

Sent with 💌 from NxCloud.

@valentinpalkovic valentinpalkovic changed the title Add option for weighted story sorting Sidebar: Add option for weighted story sorting Jul 29, 2024
@chimericdream
Copy link
Author

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".

@shilman
Copy link
Member

shilman commented Aug 8, 2024

@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

@chimericdream
Copy link
Author

@shilman no worries! I'll see if I can attend the meeting next week, but either way I'm looking forward to hearing back.

@chimericdream
Copy link
Author

@shilman how did the meeting go? Sorry I wasn't able to attend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants