Skip to content

Conversation

@KaiminHuang
Copy link
Contributor

@KaiminHuang KaiminHuang commented Jul 25, 2019

Description

The bug is originally captured by rollbar whitin Zendesk, after upgrading dropdown component @zendeskgarden/react-dropdowns": "6.2.1" .

The problem is that Google Translate replaces text nodes with tags containing translations while React keeps references to the text nodes that are no longer in the DOM tree.

React throws in the following cases:

  • A text node is conditionally rendered and it's not the only child of its parent.
  • A node before a text node is conditionally rendered

The easiest workaround is to wrap those text nodes with so that nodes referenced by React will stay in the DOM tree even though their contents are replaced with tags.

Detail

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 💅 view component styling is based on a Garden CSS
    component
  • 🌐 Styleguidist demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • ♿ analyzed via axe and evaluated using VoiceOver
  • 💂‍♂️ includes new unit tests
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

@coveralls
Copy link

coveralls commented Jul 25, 2019

Coverage Status

Coverage remained the same at 95.463% when pulling 99dc741 on kahuang/CADMIN-1768_DOMException_caused_by_google_translate into f627af1 on master.

@KaiminHuang KaiminHuang force-pushed the kahuang/CADMIN-1768_DOMException_caused_by_google_translate branch from 09eb258 to c844dc3 Compare July 25, 2019 05:12
Copy link
Member

@jzempel jzempel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Please remove the added .DS_Store file and address the minor grammar fix.

perform filtering is to control the `inputValue` and `onInputValueChange` props and
conditionally render `<Item>`s as necessary.

Due to a know issue caused by google translation extension, which replaces text node
Copy link
Member

Choose a reason for hiding this comment

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

*known

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👌

@KaiminHuang KaiminHuang force-pushed the kahuang/CADMIN-1768_DOMException_caused_by_google_translate branch from c844dc3 to 99dc741 Compare July 26, 2019 05:46
Copy link
Contributor

@austingreendev austingreendev left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the contribution.

@austingreendev austingreendev dismissed jzempel’s stale review July 26, 2019 16:23

Requested changes have been made

@austingreendev austingreendev merged commit 7650275 into master Jul 26, 2019
@austingreendev austingreendev deleted the kahuang/CADMIN-1768_DOMException_caused_by_google_translate branch July 26, 2019 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants