Skip to content

fix: Fix i8n errors by adding spans to button children #1703

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

Closed
wants to merge 7 commits into from

Conversation

yusinto
Copy link
Contributor

@yusinto yusinto commented Jun 3, 2025

Summary

https://launchdarkly.atlassian.net/browse/SC-295319

The gonfalon-fe channel receives many i8n errors like so:

ReactRenderingError: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

ReactRenderingError: The object can not be found here.

This is related to how the react dom gets removed/added by google translate on chrome. Read more about it here.


Related Jira issue: SC-295319: React i8n dom errors

@yusinto yusinto requested a review from a team as a code owner June 3, 2025 04:50
Copy link

changeset-bot bot commented Jun 3, 2025

🦋 Changeset detected

Latest commit: e57e12b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@launchpad-ui/components Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@yusinto yusinto requested review from apucacao and zmdavis June 3, 2025 04:50
Copy link

pkg-pr-new bot commented Jun 3, 2025

npm i https://pkg.pr.new/@launchpad-ui/components@1703
npm i https://pkg.pr.new/@launchpad-ui/icons@1703
npm i https://pkg.pr.new/@launchpad-ui/tokens@1703

commit: e57e12b

<ProgressBar isIndeterminate aria-label="loading" className={styles.progress} />
)}
</span>
<span>{children}</span>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrapped both ProgressBar and chidren with their own spans. I tested this locally and it seems to fix all button related i8n errors.

Copy link
Contributor

github-actions bot commented Jun 3, 2025

Size Change: +1 B (0%)

Total Size: 526 kB

Filename Size Change
packages/components/dist/index.es.js 17.5 kB -10 B (-0.06%)
packages/components/dist/index.js 18.3 kB -13 B (-0.07%)
packages/components/dist/style.css 7.93 kB +24 B (+0.3%)
ℹ️ View Unchanged
Filename Size
apps/vscode/dist/client.js 111 kB
apps/vscode/dist/server.js 261 kB
packages/box/dist/index.es.js 7.24 kB
packages/box/dist/index.js 7.8 kB
packages/box/dist/style.css 2.67 kB
packages/button/dist/index.es.js 1.89 kB
packages/button/dist/index.js 2.32 kB
packages/button/dist/style.css 3 kB
packages/core/dist/index.es.js 512 B
packages/core/dist/index.js 1.27 kB
packages/drawer/dist/index.es.js 1.76 kB
packages/drawer/dist/index.js 2.22 kB
packages/drawer/dist/style.css 497 B
packages/dropdown/dist/index.es.js 1.15 kB
packages/dropdown/dist/index.js 1.59 kB
packages/filter/dist/index.es.js 2.23 kB
packages/filter/dist/index.js 2.68 kB
packages/filter/dist/style.css 881 B
packages/focus-trap/dist/index.es.js 418 B
packages/focus-trap/dist/index.js 852 B
packages/form/dist/index.es.js 4.25 kB
packages/form/dist/index.js 4.73 kB
packages/form/dist/style.css 2.21 kB
packages/icons/dist/index.es.js 1.3 kB
packages/icons/dist/index.js 1.73 kB
packages/icons/dist/style.css 532 B
packages/menu/dist/index.es.js 3.69 kB
packages/menu/dist/index.js 4.16 kB
packages/menu/dist/style.css 872 B
packages/modal/dist/index.es.js 3.08 kB
packages/modal/dist/index.js 3.55 kB
packages/modal/dist/style.css 898 B
packages/navigation/dist/index.es.js 2.75 kB
packages/navigation/dist/index.js 3.21 kB
packages/navigation/dist/style.css 874 B
packages/overlay/dist/index.es.js 1.02 kB
packages/overlay/dist/index.js 1.42 kB
packages/popover/dist/index.es.js 3.01 kB
packages/popover/dist/index.js 3.43 kB
packages/popover/dist/style.css 529 B
packages/portal/dist/index.es.js 420 B
packages/portal/dist/index.js 835 B
packages/table/dist/index.es.js 1.01 kB
packages/table/dist/index.js 1.44 kB
packages/table/dist/style.css 700 B
packages/tokens/dist/fonts.css 183 B
packages/tokens/dist/index.css 1.47 kB
packages/tokens/dist/index.es.js 3.06 kB
packages/tokens/dist/index.js 3.1 kB
packages/tokens/dist/media-queries.css 113 B
packages/tokens/dist/themes.css 2.24 kB
packages/tooltip/dist/index.es.js 598 B
packages/tooltip/dist/index.js 1.02 kB
packages/tooltip/dist/style.css 337 B
packages/vars/dist/index.es.js 2.64 kB
packages/vars/dist/index.js 2.64 kB

compressed-size-action

Copy link
Contributor

@Niznikr Niznikr left a comment

Choose a reason for hiding this comment

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

Several visual regressions in Chromatic. I wonder if this is a worthwhile change and worth the tradeoff (more dom nodes) given it doesn't fully solve the issue as pointed out by the link you shared? That and we would have to update all the other components with similar structure as I doubt button is the only culprit.

@yusinto
Copy link
Contributor Author

yusinto commented Jun 3, 2025

I saw your commit @Niznikr thank you!

@yusinto
Copy link
Contributor Author

yusinto commented Jun 4, 2025

Closing in favor of #1705

@yusinto yusinto closed this Jun 4, 2025
@Niznikr Niznikr deleted the yus/SC-295319-React-i8n-dom-errors branch June 4, 2025 17:25
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.

3 participants