flowbite icon indicating copy to clipboard operation
flowbite copied to clipboard

Rails Turbo Frames multiple backdrops issues

Open WozniakMac opened this issue 1 year ago • 15 comments

Describe the bug When loading frame in rails with turbo it is initialising all components again which makes modals and drawers create multiple backdrops. It sometimes makes screen grey and you need to refresh the whole window.

It looks like this code is responsible for it. https://github.com/themesberg/flowbite/blob/main/src/index.turbo.ts#L31-L44 Instead of initialising only new components it initialise those which are already initialised again. And that makes multiple backdrops.

Removing it fixes the issue but it will stop initialising new components in turbo frames.

To Reproduce Steps to reproduce the behaviour: lazy turbo frame Demo: https://github.com/WozniakMac/flowbite-issue-demo

Desktop (please complete the following information):

  • OS: MacOS
  • Browser Chrome
  • Version 118.0.5993.117

WozniakMac avatar Oct 30 '23 15:10 WozniakMac

Hey @WozniakMac,

Thanks for reporting, I'll have a look tomorrow.

Cheers, Zoltan

zoltanszogyenyi avatar Oct 30 '23 20:10 zoltanszogyenyi

My first few takes on this:

  • looks like the issue is limited to the Drawer and Modal examples where we check the instances (tried with Popover, Dropdown and it works across pages and turbo load)
  • I'm going to have to take a few more days to debug this as I need to test it via a CDN import map

I'll also post a Ruby on Rails starter on GitHub with Flowbite pre-installed.

Thanks, Zoltan

zoltanszogyenyi avatar Oct 31 '23 10:10 zoltanszogyenyi

Fixed the double/multiple backdrop issue for the modal. It was an init check issue. Will be added to v2.0.1.

The situation is as follows:

  • go to homepage (modal and drawer works)
  • go to another page (modal and drawer works on those pages)
  • navigate BACK to the homepage -> only the modal and drawer backdrop will be shown

Will investigate further.

By the way, you can test the new modal fix by pinning this JS resource:

https://www.unpkg.com/[email protected]/dist/flowbite.turbo.min.js

Until a fix is provided, for all Ruby on Rails users, I recommend using the latest stable version of Flowbite pre-v2.0 as there are no differences of features other than the main Instance Manager update.

zoltanszogyenyi avatar Oct 31 '23 11:10 zoltanszogyenyi

@zoltanszogyenyi Thanks mate! It looks like this time modal stopped working and I can't open it twice :D. Only first modal open and only once. After it all modal stopped working :D. Could be because I load turbo frame after opening the modal. I'll try to dig into it deeper later. Maybe my first diagnose was not fully correct.

Btw. I think you create new package by mistake https://www.npmjs.com/package/flowbite-2.0 https://www.npmjs.com/package/flowbite

modal

WozniakMac avatar Oct 31 '23 11:10 WozniakMac

Hey @WozniakMac,

Thanks for testing - it's a new package because I'm just testing things out :)

Released one more package:

https://www.unpkg.com/[email protected]/dist/flowbite.turbo.js

This should always override the instances for the modals and drawers.

I think that if you want to dig deeper, you can fork the Flowbite project and make releases on a new repo name, something like flowbite-rails-test and then just use the automatically generated UNPKG CDN file to test on your local lib:

https://www.unpkg.com/[email protected]/dist/flowbite.turbo.min.js

All you need to do when you have forked the Flowbite lib is to run npm run build:npm and then npm publish --access=public, but make sure that you change the name of the package in package.json.

These are the files that you need to check out:

https://github.com/themesberg/flowbite/blob/main/src/index.turbo.ts (this is where we set up the turbo and frame load events) https://github.com/themesberg/flowbite/blob/main/src/dom/instances.ts (this is the main instance manager class that handles instances) https://github.com/themesberg/flowbite/blob/main/src/dom/events.ts (this is the class that sets up the events) https://github.com/themesberg/flowbite/blob/main/src/components/modal/index.ts (main modal component file where you also have initModals where we check the instances, this is the main part we need to debug) https://github.com/themesberg/flowbite/blob/main/src/components/drawer/index.ts (same as with the modal, but it's the drawer)

I've debugged this for over 3 hours and couldn't find a fix and I'm also not very experienced with Ruby on Rails, so your help here would be invaluable. Either way, I'll check this in the following days too. I'm here to help and collaborate on a fix.

You also may want to check for the FlowbiteInstances global object to see which instances are being created and overriden.

Cheers, Zoltan

zoltanszogyenyi avatar Oct 31 '23 11:10 zoltanszogyenyi

@zoltanszogyenyi Forgot to mention. This issue was on 1.8.1 too. The other issue I reported was from 2.0.0.

For now I have a workaround for both issues. I use my own package on gihub based on 1.8 with support for load-frame events turned off. It works fine because I don't need to load flowbite components in frames. https://github.com/WozniakMac/flowbite/pkgs/npm/flowbite

Tested 2.0.3. It's funny too :D

modal-2

WozniakMac avatar Oct 31 '23 11:10 WozniakMac

@zoltanszogyenyi I found out what is the issue. Every time you call initModals it adds new event listener.

https://github.com/themesberg/flowbite/blob/main/src/components/modal/index.ts#L304

Because of this after loading turbo frame on page it adds new event listener to modal button which calls show and hide on click twice or x-times turbo frames on page.

Telling you so you are not wasting time debugging. Trying to figure out how to fix it now

WozniakMac avatar Oct 31 '23 12:10 WozniakMac

@zoltanszogyenyi It looks like I fixed it 🎉 🎉 Take a look when you are free https://github.com/themesberg/flowbite/pull/700

WozniakMac avatar Oct 31 '23 15:10 WozniakMac

Hey @WozniakMac,

Appreciate the PR!

I did my own fix though, because I wanted to keep everything nice and tidy in terms of classes.

Here's my commit that fixes the issue: https://github.com/themesberg/flowbite/pull/678/commits/db93b3ed61887406823831f1f15933236f09e20e

Basically, I keep a store of all event listeners for each instance and destroy them when also destroying the instance.

The problem with your solution is that you created another instance manager when we already had one.

Please install this version and test it on your repo: https://www.npmjs.com/package/flowbite-2.1.0

This will be pushed to v2.1.0 later today or tomorrow.

Thanks! Zoltan

zoltanszogyenyi avatar Nov 09 '23 12:11 zoltanszogyenyi

@zoltanszogyenyi It works fine until it loads anything with "turbo:frame-load". Then it breaks. You can debug it on this app I prepared. Just pin it to local files with yarn add file:./../flowbite and then you need to build flowbite and run yarn add file:./../flowbite every time you change something in flowbite to see changes in rails.

https://github.com/WozniakMac/flowbite-issue-demo Just clone it. Download rbenv and install ruby. Then run ./bin/dev

I'm going to use my fork for now.

WozniakMac avatar Nov 09 '23 13:11 WozniakMac

Hey @WozniakMac ,

Thanks for the feedback.

What exactly breaks? Right now, the event listeners are being added for each new instance.

I'll take a look tomorrow - this works for my local turbo frame load repository.

Cheers, Zoltan

zoltanszogyenyi avatar Nov 09 '23 15:11 zoltanszogyenyi

Please make sure you use this package:

https://www.npmjs.com/package/flowbite-2.1.0

zoltanszogyenyi avatar Nov 09 '23 15:11 zoltanszogyenyi

I have the same problem with sidebar drawer. I tried the package you mentioned and the same issue still exists. In the first page load I got 2 messages saying add new drawerdrawer-navigation one from initDrawers2 and another one from initDrawers function (In this order). When I open the sidebar and navigate to another page I got add new drawerdrawer-navigation message from initDrawers2, then Drawer3 raises an error in this function:

Drawer3.prototype._destroyBackdropEl = function() {
  if (this._visible) {
    document.querySelector("[drawer-backdrop]").remove();
  }
};

Setting data-drawer-backdrop="false" didn't solve the problem, flowbite is always trying to remove the backdrop here:

https://github.com/themesberg/flowbite/blob/cc5320a39b42e65dde03916a66e513560d0ee363/src/components/drawer/index.ts#L86

Any ideas on how to solve this? Maybe just fix the always deleting issue, and we can use data-drawer-backdrop="false" for now until the bigger issue got resolved?

AliOsm avatar Nov 14 '23 21:11 AliOsm

To hack the issue for now, I disabled backdrops, and added <div drawer-backdrop=""></div> to the end of my body. In this way everything works fine for the drawer sidebar.

AliOsm avatar Nov 15 '23 05:11 AliOsm

I'm seeing the same issue as @AliOsm - any navigation away from a drawer link results in Cannot read properties of null (reading 'remove') for this code:

_destroyBackdropEl() {
  if (this._visible) {
    document.querySelector('[drawer-backdrop]').remove();
  }
}

Definitely related to Turbo because if you add data-turbo=false to the link then it works without error.

Great suggestion to add the empty div at the end of the body!! Hopefully we get a cleaner fix soon.

gacharles23 avatar Dec 05 '23 20:12 gacharles23