tech-systems / panes

🎉📱 Create dynamic modals, cards, panes for your applications in few steps. One instance – Thousands solutions. Any framework and free.
https://panejs.com
MIT License
687 stars 40 forks source link

[BUG] Ionic apps required await for componentOnReady() before set event listeners #216

Closed roman-rr closed 11 months ago

roman-rr commented 1 year ago

Describe the bug Something in Ionic bundle, and can be addressed to Ionic repository.

Can be fixed with:

  1. Check if events can be attached to pane before transition. Does many rely on promise inside TransitionEnd function?
  2. Or hardcode
    await (<any>document.querySelector('ion-app')).componentOnReady();
roman-rr commented 1 year ago

Addressed to https://github.com/ionic-team/ionic-framework/issues/27984

chrisk-7777 commented 11 months ago

It appears that componentOnReady is no longer available on document.querySelector('ion-app').

When trying to use this panes library, I receive the following error:

CleanShot 2023-11-17 at 16 07 49

I've read through the changelog but can't find anything to suggest that it was removed.

I'm on version 7 of ionic.

As a temporary workaround, I have stubbed out the method so this library doesn't error:

  useEffect(() => {
    (document.querySelector('ion-app') as any).componentOnReady = () => {};
    myPane.current = new CupertinoPane(panelRef.current!, {
      parentElement: 'body',
      breaks: {
        middle: { enabled: true, height: 300, bounce: true },
        bottom: { enabled: true, height: 80 },
      },
      events: {
        onDrag: () => console.log('Drag event'),
      },
    });
  }, []);
roman-rr commented 11 months ago

Hello @chrisk-7777

It seems that componentOnReady() is a stencil function. And I don't see any issues here (example) using it.

You probably won't use componentOnReady() if you do not initialize the pane instantly on page load. This function is to fix setting event assignation on page load. You might also replace it with 150ms timeout, but of course, it is not a perfect way.

I suspect that when you try to call this method inside of useEffect, the Ionic component is not loaded fully at all, and the method not accessible.

What do you think?

chrisk-7777 commented 11 months ago

Hey @roman-rr thanks for the reply!

Honestly, I'm not too sure - I'm not familiar with stencil at all. This is just a fresh ionic 7 app with only pane installed.

Sorry - to be clear, I'm not using componentOnReady directly. It seems pane uses it under the hood if it detects an ionic app in use (https://github.com/tech-systems/panes/blob/master/src/cupertino-pane.ts#L290).

If I add a breakstop to that line, and inspect this.ionApp, componentOnReady isn't on it. Even if I set a timeout of 5000ms. However, I can see other lifecycle methods, like componentDidLoad

CleanShot 2023-11-18 at 11 34 09

I've tried to hunt around the Stencil docs for componentOnReady but couldn't find anything (https://stenciljs.com/docs/component-lifecycle).

Typically useEffect would be the place to mount something like this, so I'm not positive that is the issue. It just seems that componentOnReady simply doesn't exist on document.querySelector('ion-app').

One interesting difference between your fiddle and my react version is ion-app never receives a class hydrated.

CleanShot 2023-11-18 at 11 42 26

I'm happy to continue with my workaround, it seems to work. Do you know of any react + ionic + panes simple examples available, I could be missing something painfully obvious.

roman-rr commented 11 months ago

@chrisk-7777 Thank you for details. Are you able to repository with demo code, or create environment with react and simple reproduction on jsfiddle or similar platforms?

chrisk-7777 commented 11 months ago

@chrisk-7777 Thank you for details. Are you able to repository with demo code, or create environment with react and simple reproduction on jsfiddle or similar platforms?

Yup, easy one: https://github.com/chrisk-7777/ionic-7-pane-demo

npm install
npm run dev

Then click the "open pane" button on Tab1.

There are only two commits, commit 1 is the base ionic install, commit 2 is adding pane.

Note that by uncommenting the line here then it works, because the componentOnReady method is stubbed.

roman-rr commented 11 months ago

@chrisk-7777 Thank you, I pushed fix and its available from master branch. I also think it is more Ionic issue and created new ticket

chrisk-7777 commented 11 months ago

Awesome, thanks @roman-rr !

chrisk-7777 commented 10 months ago

Sorry a bit of a delay, but can confirm that latest commit did solve the issue 👍