Skip to content
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

Modal backdrop does not cover full screen when viewport size is small #520

Closed
zammitjohn opened this issue Jan 6, 2023 · 5 comments
Closed
Labels
needs info Further information is requested

Comments

@zammitjohn
Copy link

Describe the bug
Modal backdrop does not cover full screen when viewport size is small

To Reproduce
Steps to reproduce the behavior:

  1. Open https://flowbite-react.com/modal
  2. Toggle modal
  3. Resize window size width to < 767 px

Expected behaviour
Backdrop should always cover full screen

Screenshots
Screenshot 2023-01-06 124911

@mortezasabihi
Copy link
Contributor

The bug is caused by the core flowbite found at this link: https://github.com/themesberg/flowbite/blob/e8c6490bd1893f467f347c41dc5147e6648b9dcb/plugin.js#L455.
The purpose of this line modal: 'calc(100% - 2rem)' is unclear, could you provide some context @zoltanszogyenyi?

@rluders
Copy link
Collaborator

rluders commented Jan 11, 2023

Well, we basically don't use any flowbite core javascript in the react-flowbite... So...

@zoltanszogyenyi
Copy link
Member

zoltanszogyenyi commented Jan 11, 2023

Hey peeps,

We use that to enable vertical scroll when the body content of the modal exceeds the viewport of a smaller device - it's probably not the best solution and we'll revisit this when overhauling the modal component with more options and animations.

Here's an example without the h-modal tag on mobile devices:

Screenshot 2023-01-11 at 14 27 22

As you can see, we add this to make sure that the modal takes up the height of the viewport minus 2rem to add some space between the modal and the edge of the screen.

I believe we will find a better solution when we'll update the modal component.

Cheers,
Zoltan

@rluders
Copy link
Collaborator

rluders commented Jan 12, 2023

@zoltanszogyenyi let me know when if got fixed at the flowbite core, so we can apply the fix here as well. :)

@rluders rluders added the needs info Further information is requested label Jan 12, 2023
@zoltanszogyenyi
Copy link
Member

zoltanszogyenyi commented Mar 4, 2023

Hey @rluders @tulup-conner,

I've checked the behaviour of the modal on Flowbite React and we might have to generate a <div> that will be prepended to the last tag of the </body> element and apply these basic classes:

bg-gray-900 bg-opacity-50 dark:bg-opacity-80 fixed inset-0 z-40

The gap is there because the modal might have more content inside of it so we want to enable scroll on mobile devices so the users can navigate through all of the content available inside of the modal.

We also have the h-modal class which I think that we could exchange with h-[calc(100%-1rem)].

This is probably the best solution especially if we're going to allow users to close the modal when clicking outside of the modal, I believe that the backdrop should be a separate HTML tag and it shouldn't be the modal itself, unless there are valid accessibility issues though I couldn't think of one.

Check out what happens with the DOM for the core modal component from Flowbite - one thing we should be wary of is that we only allow one backdrop at a time, meaning we have to prevent them stacking up. For example, if you have a "wizard" to switch from one modal to another these backdrops shouldn't stack up since the background will get darker and darker and eventually it'll be stuck until a refresh is hit.

Cheers,
Zoltan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs info Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants