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

fix: widget does not load when navigating on pages with view transition [CW-3249] #9443

Merged
merged 8 commits into from May 15, 2024

Conversation

scmmishra
Copy link
Member

@scmmishra scmmishra commented May 9, 2024

When visiting another page in turbo, the body is replaced completely, this makes the navigation more SPA like. However the new body does not contain any of the elements added by a client script. This causes the SDK to fail.

This PR fixes this by restoring all the elements when rendering the new body. The alternative is re-running the script, however our SDK needs more changes to support resuming, besides it causes the widget to disappear and come back, making it the inferior choice in terms of UX

Something similar happens with Astro with view transitions, this PR fixes that too

The fix

  1. Adding a unique ID to the bubble, the styles as well as the widget holder
  2. Adding a restoreWidgetInDOM method to DOMHelpers this does the work of replacing the elements as required.
  3. Adding the event listener for turbo:before-render and astro:before-swap

Things to note

While not necessary, we need to add data-turbo-eval="false" to the widget embed script, otherwise the sdk script tag will keep getting added with each navigation. It won't affect the widget.

This is not an issue with astro, since it's default behaviour is to no re-run scripts


P.S. We can remove the conditions that check if a site uses turbo or astro

@scmmishra scmmishra changed the title fix: widget does not load when navigating on pages with view transition fix: widget does not load when navigating on pages with view transition [CW-3249] May 9, 2024
Copy link

linear bot commented May 9, 2024

app/javascript/packs/sdk.js Outdated Show resolved Hide resolved
@scmmishra scmmishra temporarily deployed to chatwoot-pr-9443 May 10, 2024 03:17 Inactive
@scmmishra
Copy link
Member Author

@pranavrajs @fayazara PTAL

@scmmishra scmmishra merged commit bc8736c into develop May 15, 2024
12 of 15 checks passed
@scmmishra scmmishra deleted the fix/turbo-astro-page branch May 15, 2024 05:15
@fayazara
Copy link
Contributor

fayazara commented May 15, 2024

Quick question - is this intended? @scmmishra

CleanShot.2024-05-15.at.11.20.16.mp4

@scmmishra
Copy link
Member Author

scmmishra commented May 15, 2024

Quick question - is this intended? @scmmishra

CleanShot.2024-05-15.at.11.20.16.mp4

Yeah, once the widget remounts, the iframe is forced to render again, nothing we can do about it.

See hotwired/turbo#783

There are some implementations that require installing morphdom which didn't seem ideal

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

Successfully merging this pull request may close these issues.

None yet

4 participants