-
Notifications
You must be signed in to change notification settings - Fork 124
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
Fix unexpected closing of longer modal when clicking on scrollbar #2322
Conversation
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.
Thanks for the work @WillCWX! Some minor comments :)
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.
LGTM! Thanks for the work @WillCWX
Hi @WillCWX, wondering if you have explored ways to make it work without moving the scrollbar into the modal? Would be great if we could provide both options, perhaps something like what Chakra UI offers: https://chakra-ui.com/docs/components/modal/usage#modal-overflow-behavior |
@yucheng11122017 what do you mean by previous display? Existing modal without this PR change has a "padding" at the bottom, which is achievable if we do with max height = 100% + some other CSS. I think this might be more comfortable visually hence let's go with this instead 😺. I adjusted the style slightly to
This is not the best solution out there but I think it does address the issue for now. In fact, scrolling within the body seems to be the solution for long modals for v2 of vue-final modal (slight difference in that they have more margin) : Will post a separate issue to track modal improvements. Some investigation note:
|
@all-contributors please add @WillCWX for code |
I've put up a pull request to add @WillCWX! 🎉 |
What is the purpose of this pull request?
Fixes #2013
Fixes this of #1916 :
Overview of changes:
div class="modal-body"
.modal
tests accordingly.Anything you'd like to highlight/discuss:
The
max-height
of thediv class="modal-content"
is set to90vh
to enable scrolling asmax-height
is infinite otherwise.90vh
looks best to me but I don't have very good eyes for design so do play around with the number to see what maybe best.I have not tested scrolling in the
x-axis
as I have yet to find amodal
that is longer than the webpage but addingmax-width=90vw
may fix this easily as well.Testing instructions:
Add this topic1.md file to a markbind website.
The
modal
should have a scrollbar that is clickable and looks like this:the white bar is from snip and not from the website
Proposed commit message: (wrap lines at 72 characters)
Fix unexpected closing of longer modal when clicking on scrollbar
Fix modal scroll
Checklist: ☑️