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

[Bug] Drawer backdrop missing when bind:backdrop is false to start, even if true later #1373

Open
greysdawn opened this issue Jun 22, 2024 · 1 comment

Comments

@greysdawn
Copy link

greysdawn commented Jun 22, 2024

Describe the bug

(Sorry for the long title haha)

As the title says: when creating a drawer component with backdrop initially set to false, but later set to true through bindings, the backdrop will never display even when true. I suspect this is due to the way that the backdrop classes are initialized, as that only checks for backdrop to be true during the initialization of the component (see: this line)

Doing this the other way around works (ie. initializing to true and later changing to false), but can create unintended behaviors. An example of that can be seen with an app that keeps the drawer open on larger screens, where refreshing the page will cause the backdrop to show before being set to false and becoming hidden

Reproduction

Repl: https://replit.com/@GreyHimmel/Drawer-Backdrop-Bug?v=1

This doesn't display the other backdrop issue I mentioned above (with drawers that are always open), but it does showcase the different behaviors between the two initialization values. Should work out of the box and has buttons to activate each type of drawer and toggle their backdrop props

Flowbite version and System Info

- System:
   - OS: Windows 11 10.0.22635
   - CPU: (8) x64 Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz
   - Memory: 1.57 GB / 7.79 GB
- Binaries:
   = Node: 20.11.1 - C:\Program Files\nodejs\node.EXE
   - npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD
- Browsers:
   - Edge: Chromium (127.0.2651.8)
   - Internet Explorer: 11.0.22621.3566
- npmPackages:
   - @sveltejs/kit: ^2.0.0 => 2.5.15
   - flowbite-svelte: ^0.46.2 => 0.46.2
   - svelte: ^4.2.7 => 4.2.18
   - vite: ^5.0.3 => 5.3.1
@greysdawn
Copy link
Author

Thinking about it, I'm not entirely sure why the line I referenced above is written the way it is. The only time the backdropDivClass is used is when the backdrop is explicitly desired. I'm assuming this might be a holdover from a previous version where the backdrop was used as the outside click wrapper every time, but that doesn't seem to be the case anymore. Would it be safe to get rid of the extra class checks?

This should work fine and be safe to use from what I can tell:

// current
let backdropDivClass = twMerge("fixed top-0 start-0 z-50 w-full h-full", backdrop && bgColor, backdrop && bgOpacity);

// proposal
let backdropDivClass = twMerge("fixed top-0 start-0 z-50 w-full h-full", bgColor, bgOpacity);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant