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

Only use criteria to select mode when story first loads #218

Merged
merged 10 commits into from
Mar 15, 2024

Conversation

andrewortwein
Copy link
Contributor

@andrewortwein andrewortwein commented Mar 13, 2024

QA

  1. Use a project with stories with modes:
    • One story should have a mode that is passed and then a mode that has changed, in that order
    • Another story should have the same modes, but both modes have changes
    • A third story should have additional modes that are not found in the first two stories, with changes
  2. Navigate to the first story and verify that the second mode (the one with changes) is selected
  3. Verify that you can change to the first mode and view the test
  4. Navigate to the second story and verify that the same mode is shown from step 3
  5. Navigate back to the first story and verify the the unchanged mode is selected (this takes precedence over a selected mode that does not contain changes)
  6. Navigate to the third story and select a mode that is unique to that story
  7. Navigate back to the first story and verify that the mode with changes is selected
📦 Published PR as canary version: 1.2.22--canary.218.31120f4.0

✨ Test out this PR locally via:

npm install @chromatic-com/storybook@1.2.22--canary.218.31120f4.0
# or 
yarn add @chromatic-com/storybook@1.2.22--canary.218.31120f4.0

Comment on lines 125 to 131
play: playAll(async ({ canvasElement, canvasIndex }) => {
const canvas = within(canvasElement);
const menu = await canvas.findByRole("button", { name: "Safari" });
await userEvent.click(menu);
const items = await screen.findAllByText("Chrome");
await userEvent.click(items[canvasIndex]);
}),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this play function since useTests now selects the first non-passed test/comparison by default. But, this story's comment kind of sounds like we expected to show the first passed test by default. Since that is no longer the case, is this story still valid at all?

Copy link
Member

Choose a reason for hiding this comment

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

Let's drop the play function, instead update the code comment above and update the snapshot.

@andrewortwein
Copy link
Contributor Author

@ghengeveld I made some updates to fix some typecheck problems and left a comment on a story I had to change. Please re-review this. Thanks.

@ghengeveld ghengeveld added release Auto: Create a `latest` release when merged patch Auto: Increment the patch version when merged labels Mar 15, 2024
@andrewortwein andrewortwein merged commit 978c731 into main Mar 15, 2024
9 of 10 checks passed
@andrewortwein andrewortwein deleted the andrew/fix-unchanged-mode-selection branch March 15, 2024 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Auto: Increment the patch version when merged release Auto: Create a `latest` release when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants