-
Notifications
You must be signed in to change notification settings - Fork 350
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
Wrap text node to reduce Google Translate bug #1814
Conversation
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
Size Change: -33 B (0%) Total Size: 857 kB
ℹ️ View Unchanged
|
Hrm. I've looked at this Sentry issue many times and had no idea where it came from. I wonder if it's all down to Google Translate or other DOM manipulation tooling? |
@jeremywiebe seems likely! |
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (804b9b8) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1814 If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1814 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
let color; | ||
let backgroundColor: string | undefined; | ||
let borderColor: string; | ||
let color: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing up these type issues!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooof. Adding elements "just because" makes me sad.
Thanks for finding a simple work-around!
It looks like Google Translate doesn't play nicely with React.
There is a known issue that Google plans to address... one day, maybe.
In the short term, it looks like if we wrap text nodes in elements, we should be good to go.
Issue: LEMS-2574
Testing:
Use Google Translate on any radio widget exercise and do something that causes the radio widgets to change state to render a text node (the easiest way was showing rationales). The red bar of doom doesn't show.