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

Coverted all the css into Tailwind css of Modal component #1743

Conversation

subhajit20
Copy link
Contributor

@subhajit20 subhajit20 commented Sep 23, 2023

Description of changes

Coverted all the css into Tailwind css of Modal component
Issue - #1725

Issue Resolved

Fixes #NA

Screenshots/GIFs

@vercel
Copy link

vercel bot commented Sep 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
operation-code ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 23, 2023 8:38pm
storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 23, 2023 8:38pm

@cypress
Copy link

cypress bot commented Sep 23, 2023

1 flaky test on run #4456 ↗︎

0 35 0 0 Flakiness 1

Details:

update snapshot tests
Project: operation_code Commit: b518d21da3
Status: Passed Duration: 03:01 💡
Started: Sep 23, 2023 8:40 PM Ended: Sep 23, 2023 8:43 PM
Flakiness  cypress/e2e/modal.spec.js • 1 flaky test • all tests

View Output Video

Test Artifacts
when the server responds successfully > closes the modal when the x button is clicked Output Screenshots Video

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

components/Modal/Modal.js Outdated Show resolved Hide resolved
tailwind.config.js Outdated Show resolved Hide resolved
@kylemh
Copy link
Member

kylemh commented Sep 23, 2023

Screen.Recording.2023-09-23.at.11.12.33.AM.mov

The attached video is me trying to use the modal component on the /code_schools page on this PR's preview deployment. Nothing seems to be rendered, but click events are disabled on the body. Later in the video, I jump to operationcode.org/code_schools to prove that it's working on prod today and what it looks like. Somehow the changes here have broken the Modal.

@subhajit20
Copy link
Contributor Author

Screen.Recording.2023-09-23.at.11.12.33.AM.mov
The attached video is me trying to use the modal component on the /code_schools page on this PR's preview deployment. Nothing seems to be rendered, but click events are disabled on the body. Later in the video, I jump to operationcode.org/code_schools to prove that it's working on prod today and what it looks like. Somehow the changes here have broken the Modal.

Is this issue related to making changes in ui or any other reason ?

childrenClassName,
}) {
if (isOpen) {
gtag.modalView(screenReaderLabel);
}

const portalContainer =
Copy link
Member

@kylemh kylemh Sep 23, 2023

Choose a reason for hiding this comment

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

Hey @subhajit20 ! I dove into this one rather than spend more of your time than you bargained for...

The issue where the modal was not rendering is explained because we use '#__next' in the "important" config. Radix's Dialog component uses React Portal to render as a direct child of body rather than the div with the #__next ID (as screenshotted). We needed to specifiy the container prop on the Dialog.Portal component so that Tailwind's styles actually applied!

Screenshot 2023-09-23 at 11 38 34 PM

I also made it so Tailwind works in Storybook.

I made some other changes, but nothing you need to worry about! Great work again.

@codeclimate
Copy link

codeclimate bot commented Sep 23, 2023

Code Climate has analyzed commit b518d21 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 16.6% (90% is the threshold).

This pull request will bring the total coverage in the repository to 75.7% (0.0% change).

View more on Code Climate.

@kylemh kylemh merged commit 677ad88 into OperationCode:main Sep 23, 2023
3 checks passed
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