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: Add ability to un-accept per story #82

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Sep 6, 2023

What I did:

  • I added an extra callback function prop that invokes a mutation changing the status back to pending
  • I passed this callback down to the right component
  • In the SnapshotComparison-component I added an button that is shown when the status is APPROVED
  • The button invokes the callback when clicked

How to test

I tried this and it did not work! I need help figuring out why!

  • Open the repo storybook
  • Make a change that will be detected in chromatic
  • Start a build (clicking the button in the sidebar should do it)
  • Go to the addon's Panel
  • Wait for the change to appear
  • Approve the change
  • Wait for that to build to be updated
  • It should now show the "Unapprove" button <-- This was not working when I was testing this locally
  • Clicking the "Unapprove" button should cause the "Approve" button to appear again.

Open questions:

I suspect that there are scenarios where there's a newer build coming in where "Approve" and "Unapprove" need to be disabled. @tmeasday

📦 Published PR as canary version: 0.0.59--canary.82.de5dbe0.0

✨ Test out this PR locally via:

npm install @chromaui/addon-visual-tests@0.0.59--canary.82.de5dbe0.0
# or 
yarn add @chromaui/addon-visual-tests@0.0.59--canary.82.de5dbe0.0

@linear
Copy link

linear bot commented Sep 6, 2023

AP-3626 Add "unaccept" button

Allow resetting an accepted test back to "pending".

As discussed below, the button does not deal with batching.

Screenshot 2023-09-04 at 14.21.19.png

@ndelangen ndelangen self-assigned this Sep 6, 2023
@ndelangen ndelangen added the enhancement Classification: Improvement to existing feature label Sep 6, 2023
@ndelangen ndelangen changed the title UI: add ability to un-accept per story UI: Add ability to un-accept per story Sep 6, 2023
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @ndelangen

I suspect that there are scenarios where there's a newer build coming in where "Approve" and "Unapprove" need to be disabled. @tmeasday

Yes, I think it's part of this ticket @ndelangen https://linear.app/chromaui/issue/AP-3633/show-snapshot-outdated-header-as-soon-as-we-have-a-new-snapshot-for.

If you are interested in tackling it, it would be simply when nextBuild.id !== storyBuild?.id

@ghengeveld
Copy link
Member

@ndelangen I've updated this to hide the review button according to @tmeasday's suggestion.

@ndelangen
Copy link
Member Author

@ghengeveld or @tmeasday did either of you see that I'm blocked here?
When I tested this locally, it did not work.

The "How to test" section states where things break.
I'd appreciate it either of you could advise or assist.

@ghengeveld
Copy link
Member

It did work for me, it just took a while for the UI to update. I created a ticket for that here. Could you try again and see if you get the same effect?

@ndelangen
Copy link
Member Author

Yes, I see that too, it took about 5 second for me.

@ndelangen ndelangen merged commit b30e62e into main Sep 7, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Classification: Improvement to existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants