Skip to content

Modal tests#2377

Merged
lindapaiste merged 4 commits intoprocessing:developfrom
ofhope:modal-tests
Aug 14, 2023
Merged

Modal tests#2377
lindapaiste merged 4 commits intoprocessing:developfrom
ofhope:modal-tests

Conversation

@ofhope
Copy link
Contributor

@ofhope ofhope commented Aug 13, 2023

Fixes #issue-number

This PR adds storybook documentation and unit tests to the modal component.

Screenshot 2023-08-11 at 6 06 11 am

It also looks like a bable upgrade along the way broke storybook hot module reloading.
I didn't spend a lot of time investigating the cause but did find a solution to monkey patching the issue. HMR was still working in storybook but un-viewable without this patch.

Screenshot 2023-08-11 at 6 05 10 am

Changes:

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste
Copy link
Contributor

It also looks like a bable upgrade along the way broke storybook hot module reloading. I didn't spend a lot of time investigating the cause but did find a solution to monkey patching the issue. HMR was still working in storybook but un-viewable without this patch.

THANK YOU! I've been seeing that error but hadn't had a chance to look into it yet.

Copy link
Contributor

@lindapaiste lindapaiste left a comment

Choose a reason for hiding this comment

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

Looks great! I love the tests for the inside and outside click behavior and I so grateful for the window.$RefreshReg$ fix.

@lindapaiste
Copy link
Contributor

It's a bummer that it doesn't render with the proper styling. There's two issues there, one of which I only just now realized! Maybe you can fix it in a separate PR.

  1. The modal needs to have a parent with class="light" (or "dark" or "contrast"). This will be fixed by Add theme switching to Storybook #2326, but we can also put in a quick fix until that gets merged since it might be a while. We can add a <div className="light"> to the decorators in preview.js.
  2. The compiled main.css file that we are importing in preview.js is not up to date. I don't know what that file is for and probably it should be deleted. We want to import the uncompiled file import '../client/styles/main.scss' as that will be current will all changes. But in order to do that we also need a Storybook addon to process it: https://storybook.js.org/addons/storybook-addon-sass-postcss

@lindapaiste lindapaiste merged commit 62003a9 into processing:develop Aug 14, 2023
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