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

305 update chromatic configurations #325

Merged
merged 32 commits into from
Mar 13, 2024

Conversation

ekraffmiller
Copy link
Contributor

@ekraffmiller ekraffmiller commented Feb 25, 2024

What this PR does / why we need it:

The Chromatic snapshots are hard to review because they show changes that are due to faker randomized data, and some stories have built in loading delays. The changes make the faker data more reproducible, and add a delay before taking some snapshots.

Which issue(s) this PR closes:

Special notes for your reviewer:

Suggestions on how to test this:

To reproduce the behavior of a Chromatic build, add a.envfile to the root directory of the project, with this setting:
STORYBOOK_CHROMATIC_BUILD=true
In a local build, open Storybook and go to a story. If you refresh the browser multiple times, the faker data should stay the same between refreshes.
The link to the Chromatic build is in the github action. You can go to the build, and approve the current changes (there have been ui changes since the last approval), and check that the faker data doesn't change between builds.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

@ekraffmiller ekraffmiller linked an issue Feb 25, 2024 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Feb 25, 2024

Coverage Status

coverage: 97.179%. remained the same
when pulling a9f9fda on 305-update-chromatic-configurations
into 0e90218 on develop.

@cmbz cmbz added the Size: 10 A percentage of a sprint. 7 hours. label Feb 26, 2024
@ekraffmiller ekraffmiller added the pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows label Feb 26, 2024
@M27Mangan
Copy link

@ekraffmiller - Chromatic works as-intended now, its a marvelous thing!
Just need to fix the merge conflicts and we're good to go!

Copy link

@M27Mangan M27Mangan left a comment

Choose a reason for hiding this comment

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

Finishing file review and requesting changes

@M27Mangan M27Mangan assigned ekraffmiller and unassigned M27Mangan Feb 29, 2024
@ekraffmiller
Copy link
Contributor Author

Hi @M27Mangan, I merged the latest from develop, and you can see that several snapshots have changed now...I think that's because the new collection stories changed the order in which faker data was generated. I experimented with setting the faker seed in every .stories file to avoid this, but didn't have much luck. So this is a compromise, and sometimes the data will change. I think if we want to pursue it more, we could create another issue, let me know what you think.

M27Mangan
M27Mangan previously approved these changes Mar 5, 2024
@M27Mangan
Copy link

👍 Approved!

@M27Mangan M27Mangan removed their assignment Mar 5, 2024
@MellyGray MellyGray self-assigned this Mar 11, 2024
@MellyGray
Copy link
Contributor

Hi @M27Mangan, I merged the latest from develop, and you can see that several snapshots have changed now...I think that's because the new collection stories changed the order in which faker data was generated. I experimented with setting the faker seed in every .stories file to avoid this, but didn't have much luck. So this is a compromise, and sometimes the data will change. I think if we want to pursue it more, we could create another issue, let me know what you think.

Hi @ekraffmiller, have you resolved the issue? I reviewed the latest build and noticed some changes in the snapshots

image

@ekraffmiller
Copy link
Contributor Author

@MellyGray I updated the code to remove the loading delay, because I wanted to make sure all the calls to faker were made in a deterministic way. This removed most of the differences, but there are still some differences in FileInfoCell that I need to figure out. So I'm going to put the issue back to In Progress

@MellyGray MellyGray removed their assignment Mar 12, 2024
Copy link
Contributor

@MellyGray MellyGray left a comment

Choose a reason for hiding this comment

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

Looks good! The Chromatic builds seem to be fixed!

@MellyGray MellyGray merged commit 1db6688 into develop Mar 13, 2024
14 checks passed
@MellyGray MellyGray deleted the 305-update-chromatic-configurations branch March 13, 2024 09:46
@MellyGray MellyGray removed their assignment Mar 13, 2024
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
…ions

305 update chromatic configurations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Chromatic configurations
5 participants