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

Fix issue with switching build without changing story #130

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Oct 12, 2023

Telescoping from #124

We had a bug where it automatically switched you to the new build when you already had a selected build as soon as you captured a new story. The specified behaviour was that it should "hold" until you opt-in (by clicking the header) or change story.

This ensures that (and adds a story that tests the changing story part actually works)...

To QA:

  1. Open a branch with a build, view a snapshot
  2. Run another build, let it finish
  3. Ensure you are still viewing the old snapshot (it should say "View latest snapshot" in the header, or check the build # in the footer).
  4. Change story
  5. Check you are now viewing the new build.
📦 Published PR as canary version: 0.0.110--canary.130.339125d.0

✨ Test out this PR locally via:

npm install @chromaui/[email protected]
# or 
yarn add @chromaui/[email protected]

In terms of replacing the selectedBuild with the lastBuildOnBranch and switching selectedBuilId
Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

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

With more and more of this kind of view logic accumulating, maybe it's time to invest in some structured state management? Anyway, this looks good.

@tmeasday tmeasday merged commit 339125d into tom/ap-3703-auto-switch-to-latest-if-story-is-on-latest-build-but-not-on Oct 27, 2023
@tmeasday tmeasday deleted the tom/fix-auto-changing-story branch October 27, 2023 01:29
@tmeasday
Copy link
Member Author

For me it's less about the state management (the file that transitions is pretty simple really), and more about how to test it. Unit tests are better than nothing but don't really cut it here.

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

Successfully merging this pull request may close these issues.

2 participants