Fix current instabilities with Google Translate

aaemnnosttv commented 1 year ago

Feature Description

As a product used by millions globally, it's no surprise that some users use Google Translate on Site Kit interfaces. As identified in the past, this can result in fatal application errors when React attempts to rerender and gets "confused" due to DOM manipulations made by Google Translate.

In our error reporting (for users who opt-in – thank you 😄), we can see a number of similar errors that are indicative of this kind of this problem:

e.g.

Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

Related issues

3636
2280

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

SK screens should be audited and updated for compatibility with Google Translate, specifically for cases where the dashboard crashes due to an update after translation (i.e. Failed to execute 'removeChild' on 'Node')

Implementation Brief

In assets/js/components/dashboard-sharing/DashboardSharingDialog/index.js:
- Define a new variable that will hold the current dynamic text, and assign it based on the existing conditions. By default set it to empty string.
- Render text from new variable to the referenced code, instead of dynamically
- Define another variable for the dynamic text in paragraph as well, and do the same as previously for h2 text
Other components work, but in just case do an overall check if there is similar error when GTranslate extension is active

Test Coverage

No updates needed

QA Brief

Changelog entry

jamesozzie commented 1 year ago

With Site Kit 1.110.0 I encountered errors documented in this task.

Dashboard Sharing: issues when closing the popup if translations were active on the popup (screenshot)
Hovering over the SK admin toolbar once a page is already translated: formatting and console error - screenshot. Possibly not unexpected, with the content here loaded after the initial page load. The issue doesn’t occur instantly. To reproduce this, use the GTranslate plugin to load a front end URL, change the language, then change the language once more before viewing the SK admin toolbar.
Hovering over the “Create a conversion” button (Chrome extension only): Sporadic console errors and formatting on the tooltip. (screenshot). I was able to recreate this 2 or 3 times but not consistently. On one occasion I switched the reporting period after translating the page, then waited until the tooltip occurred. I couldn’t repeat this from further attempts. Note that I did encounter this elsewhere also.

Happy to conduct further testing if required.

aaemnnosttv commented 11 months ago

@bethanylang this is probably best continued outside of GH since this is essentially a task to inform new issues. Let's close this and pick up in a new task.

mxbclang commented 11 months ago

Thanks @aaemnnosttv! I checked the related Asana task and it looks like back in October you had asked for the issues that James identified to be added here in Github and we closed the Asana task. What's the next step here?

aaemnnosttv commented 11 months ago

Thanks @bethanylang – we want to fix the issues with Google Translate that we're aware of, so we can redefine this issue or close it an open a new one to address these.

To be clear, we're looking to fix issues where the dashboard crashes (e.g. https://imgur.com/z3Mli0u) – we're not sure that the other examples shared (console errors) were from SK or not, but those aren't the issue we're looking to fix.

I'll update the issue so this can move forward.

zutigrm commented 11 months ago

Couldn't reproduce this error. Tried translating on main dashboard when all data is loaded, and before it is loaded. Tried the same on single page view as well

jamesozzie commented 10 months ago

@zutigrm I added some details from testing this today. You'll find details on this in this Asana task.

In my case I was only able to recreate issues as I was previously in one case, when using the Google Translate chrome extension and then closing the Dashboard Sharing modal. Details in that task.

mxbclang commented 10 months ago

@zutigrm Reassigning to you to review James' update, please. If you don't feel like you're the best person to do the IB, feel free to unassign yourself. :)

zutigrm commented 10 months ago

@bethanylang Thanks, it seems I missed the previous comment

eugene-manuilov commented 10 months ago

@zutigrm, I am not sure that your IB changes anything. Previously, in the past, we needed to wrap a text into a span to make it work correctly when Google Translate was used (see #3636). I think we probably need to do the same in this ticket as well, right?

cc @aaemnnosttv or @tofumatt in case they want to add something.

zutigrm commented 10 months ago

@eugene-manuilov The problem was when two text content are rendered conditionally, I did I quick test of the proposed implementation, it fixed the issue, so either approach should be valid.

But let's see if @aaemnnosttv or @tofumatt have something to add

tofumatt commented 6 months ago

Usually we've added the span instead, I'd rather us stick with a consistent implementation, which would be using a span, but as long as it fixes it I don't have anything to add here.

google / site-kit-wp