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

Update useSyntheticChange to only call execCommand if input is focused #3562

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

iansan5653
Copy link
Contributor

useSyntheticChange powers all updates to the MarkdownEditor textarea under the hood. Using this hook instead of directly updating the input value allows us to preserve the caret position and browser undo stack. However, it uses execCommand which has all sorts of gotchas since it essentially just simulates the user typing in the input. This means the input needs to be focused, which is fine because it usually is when you are editing Markdown.

When uploading a file, the delay between interaction and update may be significant. In this case, the user might have started working on something else. For example, they might have opened a dialog which activated a focus trap. Then we can't pull focus into the editor, so we can't use execCommand to update the input.

Currently in this case we still call execCommand, unexpectedly updating whatever input is focused instead. This is obviously not great since it means we may insert file URLs into random spots. So instead, this PR updated the hook to ensure the input is focused before trying to call execCommand. If not, it falls back to the already-available fallback method of directly updating the input.

Closes #3548

Merge checklist

  • [ ] Added/updated tests
    • This change cannot be tested because execCommand is not available in JSDom so we always use the fallback method in the test suite anyway.
  • [ ] Added/updated documentation
  • Changes are SSR compatible
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@iansan5653 iansan5653 requested review from a team and joshblack July 25, 2023 18:46
@changeset-bot
Copy link

changeset-bot bot commented Jul 25, 2023

🦋 Changeset detected

Latest commit: 210a6ac

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

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 25, 2023

size-limit report 📦

Path Size
dist/browser.esm.js 102.44 KB (+0.03% 🔺)
dist/browser.umd.js 103.04 KB (+0.04% 🔺)

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.

Uploading an item puts the URL into whatever input has focus
2 participants