-
Notifications
You must be signed in to change notification settings - Fork 8
feature: add prop to set modal location to topleft, topright, bottoml… #566
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
1e60aaf
803e88a
902a54a
66c8752
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,8 @@ import PropTypes from 'prop-types'; | |||||||||||||||||
|
|
||||||||||||||||||
| Modal.propTypes = { | ||||||||||||||||||
| children: PropTypes.node, | ||||||||||||||||||
| isOpen: PropTypes.bool | ||||||||||||||||||
| isOpen: PropTypes.bool, | ||||||||||||||||||
| modalLocation: PropTypes.string | ||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
|
|
@@ -19,7 +20,7 @@ Modal.propTypes = { | |||||||||||||||||
| * @param {React.ReactNode} props.children - The content to be displayed inside the modal. | ||||||||||||||||||
| * @param {boolean} props.isOpen - If the modal is open (visible) or not. | ||||||||||||||||||
| */ | ||||||||||||||||||
| function Modal({ children, isOpen }) { | ||||||||||||||||||
| function Modal({ children, isOpen, modalLocation }) { | ||||||||||||||||||
|
|
||||||||||||||||||
| /** Boolean for controlling the "full" CSS class modifier */ | ||||||||||||||||||
| const [fullHeight, setFullHeight] = useState(false); | ||||||||||||||||||
|
|
@@ -48,7 +49,7 @@ function Modal({ children, isOpen }) { | |||||||||||||||||
| }, [contentRef]); | ||||||||||||||||||
|
|
||||||||||||||||||
| return <div ref={modalRef} | ||||||||||||||||||
| className={`modal ${isOpen ? 'modal--open' : ''} ${fullHeight ? 'modal--full' : ''} ${kioskLocation ? 'modal--kiosk' : ''}`} | ||||||||||||||||||
| className={`modal ${modalLocation} ${isOpen ? 'modal--open' : ''} ${fullHeight ? 'modal--full' : ''} ${kioskLocation ? 'modal--kiosk' : ''}`} | ||||||||||||||||||
| > | ||||||||||||||||||
|
Comment on lines
51
to
53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid "undefined" CSS class and skip location class in kiosk Prevent rendering Apply: - return <div ref={modalRef}
- className={`modal ${modalLocation} ${isOpen ? 'modal--open' : ''} ${fullHeight ? 'modal--full' : ''} ${kioskLocation ? 'modal--kiosk' : ''}`}
- >
+ const locationClass = !kioskLocation && modalLocation ? modalLocation : '';
+ return <div ref={modalRef}
+ className={`modal ${locationClass} ${isOpen ? 'modal--open' : ''} ${fullHeight ? 'modal--full' : ''} ${kioskLocation ? 'modal--kiosk' : ''}`}
+ >📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| <div ref={contentRef} className="modal__content"> | ||||||||||||||||||
| {children} | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default not applied: modalLocation can be undefined
defaultProps.modalLocationis declared but not used when neither prop nor URL param is set. This breaks positioning after base offsets were removed.Apply:
Also applies to: 185-186
🤖 Prompt for AI Agents