-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Combined Modal component for New File, New Folder, and Upload. #2049
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
Conversation
Two notes re: close on outside click.
We can change it to There is another approach
|
@raclim I fixed note 1 with the |
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.
Hi @lindapaiste, thanks so much for working on this—sorry it took a while to revisit it! These updates makes sense to me, and I also think it looks good to go!
Since we just created a new release I'd like to wait for a moment before addressing more PRs as a precaution, so this will probably be merged in by the end of the week!
@raclim I realized why we did not have the click-outside behavior on the upload modal previously. It's because clicking the "Drop files here or click to use the file browser" section is considered an outside click and therefore closes the modal. The I can fix this by making it so that the hidden input is inside of the modal Please do not push this production until we merge in a fix. |
Sounds good, thanks for catching this—I was just about to push the release 😂 I'll hold off until then |
Changes:
The components for
NewFileModal
,NewFolderModal
andUploadFileModal
already use the same CSS class names for styling. This PR moves the HTML structure of the modal and the click-outside logic (added in #1602) into a common sharedModal
component. Any differences between the three are handled by passing props to theModal
, such astitle
andcloseAriaLabel
.I have verified that this pull request:
npm run lint
)npm run test
)develop
branch.Fixes #123