Skip to content

[LiveComponent] Release the loading state before any redirect. #2689

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

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

sukei
Copy link

@sukei sukei commented Apr 10, 2025

Q A
Bug fix? yes
New feature? no
Docs? no
Issues
License MIT

This PR attempt to release the loading state before redirecting the user. This one may seems a little weird at first, but it has its use case.

I just write an export feature that uses a LiveComponent action to build a file. While the export is made, the UI add some classes and/or attributes to convey the "in progress" meaning. When it's done, the user is redirected to a specific controller that initiate the download. Since the redirected page just provide a file attachment, the user stay on the original page (fully expected). But, at this stage, the loading state is never released. By releasing the loading state before the redirect, the UI is made responsive again.

@carsonbot carsonbot added Bug Bug Fix Status: Needs Review Needs to be reviewed labels Apr 10, 2025
@sukei sukei changed the title Release the loading state before any redirect. [LiveComponent] Release the loading state before any redirect. Apr 10, 2025
Copy link

📊 Packages dist files size difference

ℹ️ No difference in dist packagesFiles.

Comment on lines +2155 to 2158
this.hooks.triggerHook('loading.state:finished', this.element);
if (backendResponse.response.headers.get('Location')) {
if (this.isTurboEnabled()) {
Turbo.visit(backendResponse.response.headers.get('Location'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want to trigger the "finished" event before Turbo does its work.

Here you could do something just before the

window.location.href = backendResponse.response.headers.get('Location') || '';

Copy link
Author

@sukei sukei Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use Turbo on this project and I need it to be triggered in that setup too. BTW, I'm actualy using this patch and it didn't seems to create any issues.

EDIT : I disabled Turbo in that case to avoid a double fetch of the URL, that may be the reason I didn't have any issue. So WDYT? Should I move the call?

@Kocal Kocal added Status: Waiting Feedback Needs feedback from the author and removed Status: Needs Review Needs to be reviewed labels Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix LiveComponent Status: Waiting Feedback Needs feedback from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants