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] Very slow initialization #186

Closed
iamjoshua opened this issue Jul 21, 2022 · 7 comments
Closed

[BUG] Very slow initialization #186

iamjoshua opened this issue Jul 21, 2022 · 7 comments
Assignees
Labels
enhancement Extra attention is needed

Comments

@iamjoshua
Copy link

Describe the bug
When first initializing a pane, there is a delay that exceeds the threshold typical for "fast" UX which feels quite sluggish. Hiding a pane instead of destroying it will allow for subsequent presentations to be noticeably faster. But even if a pane is initialized prior to being shown, there is still an initial delay. And it seems that the default behavior for the 'onDidDismiss' event triggered by dragging events destroys the pane. I don't see an option for hiding instead of destroying on the default dismiss event.

To Reproduce
Steps to reproduce the behavior:
Any of the demos.

Expected behavior

  • A shorter delay on initial presentation
  • Initialization to actually initialize to avoid delay once presentation is called
  • An option to override destroy on dismiss events in favor of hiding.
@roman-rr roman-rr self-assigned this Jul 21, 2022
@roman-rr roman-rr added the enhancement Extra attention is needed label Jul 21, 2022
@roman-rr
Copy link
Collaborator

@iamjoshua Thank you for issue. Please show up your initial pane settings.

@iamjoshua
Copy link
Author

I have no specific settings in mind. The default examples that you provide exhibit the behavior. Adding/Removing elements from the DOM will always be slower than toggling a css class. For performance, showing/hiding should be the default behavior instead of init/destroying.

@roman-rr
Copy link
Collaborator

roman-rr commented Aug 8, 2022

@iamjoshua Just pushed 1.3.01 version where fitHeight was improved.
If you are using this options, initialization time should be reduced.
If your pane contains a picture elements <img />, don't hesistage to set exact height <img height="200" />, that's will also reduce initialization time.

Please give a feedback, whether you are using fitHeight option or not.

@iamjoshua
Copy link
Author

Yes I was using the fitHeight option. The new version does seem a bit faster, but there is still a noticeable delay. I only have text in the pane so it's hard to see why there should be any delay. I still don't understand why you are destroying the pane upon dismiss by default. The default should be to hide the pane without destroying it. Or at least it would be a good option to have something like { onDismissType: 'hide' || 'destroy' }

To reiterate the original point, I can set the onBackdropTap event to hide the pane, which keeps it from being destroyed ensuring a fast reopen in the future. But when dragging the pane closed it seems to destroy the pane by default instead of simply hiding it. Is there any way currently to simply hide the pane on dragging to close?

@roman-rr
Copy link
Collaborator

@iamjoshua
Thank you for feedback. I will more investigate the nature of this delay.
Destroy make a DOM clear, and make possible to re-create pane with other options.
Maybe this is a good way to make hide by default. I'll check.

Could you please, make some demo/video to demonstrate a delay ?

@iamjoshua
Copy link
Author

That level of DOM manipulation is always going to be slower than changing a class/style to hide/show. I understand the need to clean up after using a temporary pane, but I'd have to imagine that a major use case for panes is for permanent UI elements.

Your own demo shows the issue: https://output.jsbin.com/fuhisey
To see the difference, present the pane, then destroy, then present, etc.. Next hide instead of destroying, present, hide, present, hide, etc. Notice that there is a lag before presenting after destroying but not when presenting after hiding. This lag will increase with the complexity of the content within. An option to make the default behavior to hide instead of destroy would avoid this issue. I would also add that the options "bottomClose" and "fastSwipeClose" are ambiguous between destroying and hiding so that one is forced to accept the destroy behavior by using these typically necessary behaviors. A super easy solution for everything I've said is to have an option like { closeBehavior: 'hide' || 'destroy' }

@roman-rr
Copy link
Collaborator

@iamjoshua Big Thanks for your effort, and I'm eventually come out with solution and now initialization reduced from 150ms to 10ms in average.

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

No branches or pull requests

2 participants