10-modal-development #49
Open
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
Aeri4a
requested changes
Feb 27, 2024
| return ( | ||
| <div className={styles.modalBackground}> | ||
| <div className={styles.modalContainer}> | ||
| const ModalWidget = ({header, paragraph, cancelText, confirmText, onConfirm,}) => { |
There was a problem hiding this comment.
There is unnecessary comma after last prop
Comment on lines
+7
to
+10
| const [openModal, setOpenModal] = useState(true); | ||
| const switchModal = () => { | ||
| setOpenModal(!openModal); | ||
| } |
There was a problem hiding this comment.
openModal should be passed as a prop "open" and then we will be able to control the state outside the component
| <button className={`${styles.modalButton} ${styles.confirm}`}>Confirm</button> | ||
| </div> | ||
| </div> | ||
| document.body.addEventListener('keydown', pressEscape); |
There was a problem hiding this comment.
Please, move it to useEffect function, but not the returned one, above return.
|
|
||
| return ( | ||
| <div> | ||
| <button onClick={switchModal}> Show modal </button> |
There was a problem hiding this comment.
If we will have open prop state, then we don't have to have a button here :)
Comment on lines
+42
to
+45
| <button className={`${styles.modalButton} ${styles.cancel}`} onClick={switchModal}> | ||
| {cancelText} | ||
| </button> | ||
| <button className={`${styles.modalButton} ${styles.confirm}`} onClick={pressConfirm}> |
There was a problem hiding this comment.
onClick can just include function which will be passed and we don't have to wrap it with another one
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.
No description provided.