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

Prevent pressing escape when editing document name to bubble up to the editor #3725

Merged
merged 1 commit into from May 19, 2024

Conversation

MitjaBezensek
Copy link
Contributor

@MitjaBezensek MitjaBezensek commented May 8, 2024

This prevents pressing escape to bubble to up to editor when editing document names. Prevents the current tool to change back to select tool.

Before

Pressing escape when editing the name stops the editing, but also switches from hand tool to select tool.

CleanShot.2024-05-08.at.15.37.33.mp4

After

We no longer switch to hand tool when we press escape the first time. The second time it still works though.

CleanShot.2024-05-08.at.15.51.09.mp4

@mimecuvalo happy to wait for your focus management PR to get merged, then update accordingly by using editor.focus().

Change Type

  • sdk — Changes the tldraw SDK
  • dotcom — Changes the tldraw.com web app
  • docs — Changes to the documentation, examples, or templates.
  • vs code — Changes to the vscode plugin
  • internal — Does not affect user-facing stuff
  • bugfix — Bug fix
  • feature — New feature
  • improvement — Improving existing features
  • chore — Updating dependencies, other boring stuff
  • galaxy brain — Architectural changes
  • tests — Changes to any test code
  • tools — Changes to infrastructure, CI, internal scripts, debugging tools, etc.
  • dunno — I don't know

Test Plan

  1. Edit document name.
  2. Press escape. It should stop editing the document name, but should not switch the active tool to select tool.
  3. Pressing escape once again should do it though. Also keyboard shortcuts should also work.
  • Unit Tests
  • End to end tests

Release Notes

  • Prevent escaping out of editing the document name to switch the active tool to select tool.

@huppy-bot huppy-bot bot added dotcom improvement Improves existing features labels May 8, 2024
Copy link

vercel bot commented May 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
examples ✅ Ready (Inspect) Visit Preview May 8, 2024 1:53pm
1 Ignored Deployment
Name Status Preview Updated (UTC)
tldraw-docs ⬜️ Ignored (Inspect) Visit Preview May 8, 2024 1:53pm

Copy link
Collaborator

@mimecuvalo mimecuvalo left a comment

Choose a reason for hiding this comment

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

lg! approving modulo one minor comment on focus :P

// revert to original name instantly so that when we blur we don't
// trigger a save with the new one
setState((prev) => ({ ...prev, name: null }))
inputRef.current?.blur()
editor.getContainer().focus()
Copy link
Collaborator

Choose a reason for hiding this comment

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

(i've been staring at focus too long). i don't think we need this line - looks like just doing stopEventPropagation will do the trick 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it, but then the second escape did not switch back to the select tool 🤷 Keyboard shortcuts worked, but escape didn't.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ahh interesting interesting, didn't realize that! can you add a note that that's what the focus does? definitely we'll forget why we put that there later :)

@steveruizok steveruizok added this pull request to the merge queue May 19, 2024
@steveruizok
Copy link
Collaborator

lgtm!

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 19, 2024
@steveruizok steveruizok added this pull request to the merge queue May 19, 2024
Merged via the queue into main with commit 4adbc76 May 19, 2024
10 checks passed
@steveruizok steveruizok deleted the mitja/stop-escape-propagation branch May 19, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotcom improvement Improves existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants