Add modal prop to Dialog
#907
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lyzadanger
commented
Mar 7, 2023
| @@ -14,16 +15,23 @@ import type { PanelProps } from '../layout/Panel'; | |||
| type ComponentProps = { | |||
| /** | |||
| * Dialog _should_ be provided with a close handler. We have a few edge use | |||
| * cases, however, in which we need to render a "non-closeable" modal. | |||
| * cases, however, in which we need to render a "non-closeable" modal dialog. | |||
There was a problem hiding this comment.
Fix requested in last PR.
Comment on lines
+30
to
+33
| /** | ||
| * Make this Dialog modal. Modal dialogs are: | ||
| * - Rendered with a full-screen overlay backdrop | ||
| */ |
There was a problem hiding this comment.
This comment list will get longer and be adjusted as more implementation is done...
Comment on lines
+135
to
+150
| { | ||
| // Maximum width and height should not exceed 90vw/h | ||
| 'max-w-[90vw] max-h-[90vh]': modal, | ||
| // Overlay sets up a flex layout centered on both axes. For taller | ||
| // viewports, remove this modal container from the flex flow with | ||
| // fixed positioning and position it 10vh from the top of the | ||
| // viewport. This feels more balanced on taller screens. Ensure an | ||
| // equal 10vh gap at the bottom of the screen by adjusting max-height | ||
| // to `80vh`. | ||
| 'tall:fixed tall:max-h-[80vh] tall:top-[10vh]': modal, | ||
| }, | ||
| { | ||
| // Max-width rules will ensure actual width never exceeds 90vw | ||
| 'w-[30rem]': width === 'sm', | ||
| 'w-[36rem]': width === 'md', // default | ||
| 'w-[42rem]': width === 'lg', | ||
| 'w-[30rem]': modal && width === 'sm', | ||
| 'w-[36rem]': modal && width === 'md', // default | ||
| 'w-[42rem]': modal && width === 'lg', |
There was a problem hiding this comment.
This application of sizing styles will be refined as implementation continues, with the introduction of some of the customization controls presented last week. But for now: Only assert size and positioning for modal Dialogs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR starts the division of
Dialoginto modal and non-modal sub-variants. It is possible that one day we will peel this into two components, but for now, one component that can have modal or non-modal behaviors.More behavior differentiation will be applied to modal vs. non-modal Dialogs, but for now, modal Dialogs get a full-screen backdrop Overlay, sizing and positioning, while non-modal Dialogs do not. A modal example has been added to the
Dialogpage.I've also added some next-step TODOs here on
Dialog's pattern-library page to sketch out where this is going next.Next step: some hooks to handle events that should close modal
Dialogs.Part of #77