Modal component #35
Modal component #35
Conversation
|
Visit the preview URL for this PR (updated for commit c8ec35e): https://mmp-site-b1c9b--pr35-inputs-modal-x3vlwrh9.web.app (expires Tue, 19 Mar 2024 20:39:05 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 4eb870c89e876f1812e204af417359065d2a23b1 |
Co-Authored-By: Galen Winsor <[email protected]>
| "@types/react": "^18.2.64", | ||
| "@types/react-dom": "^18.2.21", |
There was a problem hiding this comment.
Should these be dev dependencies? Or does astro want this stuff installed as a regular dependency?
There was a problem hiding this comment.
I added these using "astro add" -- "Astro includes an astro add command to automate the setup of official integrations. "
They seem to want them as dependencies, although we could manually install them as dev dep if needed?
The docs: https://docs.astro.build/en/guides/integrations-guide/react/
There was a problem hiding this comment.
Let's not fight astro then. I guess if astro is building everything keeping things as dependencies doesn't really matter
|
|
||
| return ( | ||
| <Dialog.Root open={isOpen} onOpenChange={setIsOpen}> | ||
| <Dialog.Trigger asChild> |
| className="fixed top-0 left-0 right-0 bottom-0 bg-gray-500 bg-opacity-75 transition-opacity z-10 w-screen overflow-y-auto" | ||
| onClick={() => setIsOpen(false)} | ||
| /> | ||
| <Dialog.Overlay className="fixed top-0 left-0 right-0 bottom-0 grid place-items-center z-10 w-screen overflow-y-auto"> |
There was a problem hiding this comment.
Do you need the double overlay? Why are there two overlay siblings?
There was a problem hiding this comment.
In order to have a scrollable overlay, radix recommends moving the modal content within the overlay. I still wanted to be able to click on the overlay to be able to close the modal, which is where the first one on line 19 comes in.
There was a problem hiding this comment.
and you can't use the onClick handler on the overlay on 23?
| <CustomTextarea label={"description"} placeholder={"Why you need this file"} /> | ||
| </div> | ||
| <div className="flex flex-row gap-2"> | ||
| <button className="flex items-center gap-2 rounded-lg px-5 py-2.5 bg-black text-white text-sm text-center font-medium hover:bg-gray-500 focus:ring-4 focus:outline-none focus:ring-gray-500"> |
There was a problem hiding this comment.
One last thing, and this doesn't matter a lot until we have designs, but global styles like button styles should go in a global stylesheet so you're not repeating styles.
This PR does not include any file logic -- just the components needed for the visual of the download modal. Pressing "validate email" does nothing and is expected.
Added
Components
Custom Input
Custom Textarea
Download Modal
Tools / Config
Added react compatibility
Added Radix UI Dialog
Changed
The github actions need to enable web frameworks for firebase to deploy properly; see https://docs.astro.build/en/guides/deploy/google-firebase/
Therefore, I added the webframework env variables to both github actions.
Questions for Reviewers
Do we want any color here? Please throw out any design feedback!
What do we think about having the Submit button within the modal be a ghost button until the email is validated? Right now, the button just isn't there.