Skip to content

Adding some Costume Editor Keyboard Shortcuts - #8000

Merged
WorldLanguages merged 12 commits into
ScratchAddons:masterfrom
misaunde:costume-editor-shortcuts
Jan 6, 2025
Merged

Adding some Costume Editor Keyboard Shortcuts#8000
WorldLanguages merged 12 commits into
ScratchAddons:masterfrom
misaunde:costume-editor-shortcuts

Conversation

@misaunde

@misaunde misaunde commented Dec 15, 2024

Copy link
Copy Markdown
Contributor

Adding a "costume-editor-shortcuts" add-on which adds keyboard shortcuts to the main tools in the paint editor.

Hi, first timer here 👋
My son does a lot of scratch, and he thought it'd be useful to have keyboard shortcuts in the costume editor.
Anyways, I thought I'd attempt it as a Christmas present 🎄

Resolves #3414

Changes

shortcuts_light_2 shortcuts_dark image
shortcuts.demo.mp4

Reason for changes

Keyboard shortcuts can help make switching between paint tools easier and is pretty standard in most modern painting programs.

Tests

Tested in both Chrome and Firefox.

@Samq64 Samq64 added type: enhancement New feature for the project new addon Related to new addons to this extension. `scope: addon` should still be added. scope: addon Related to one or multiple addons labels Dec 16, 2024

@Samq64 Samq64 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The shortcut indicators need to be re-added when switching between the vector and bitmap modes but everything else looks good.

Also, congratulations on getting #8000!

Comment thread addons/costume-editor-shortcuts/addon.json Outdated
Comment thread addons/costume-editor-shortcuts/addon.json Outdated
isInitialized = true;
document.addEventListener("keydown", handleKeyDown);
document.addEventListener("focusin", userStartedTyping);
document.addEventListener("focusout", userStoppedTyping);

@Samq64 Samq64 Dec 16, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I haven't tested my suggestion but I think if you add the listeners to an element in the costume editor you won't have to clean them up since Scratch will remove the elements when switching tabs anyway.

@misaunde misaunde Dec 16, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried this initially, but had issues getting anything to fire when the handlers were attached to any of the costume editor divs (no matter where I clicked). Probably user error, but document just worked.

Here's an example of what I tried:

  // var gui = await addon.tab.waitForElement("[class*='gui_tab-panel']");
  // var gui = await addon.tab.waitForElement("[class*='paint-editor_editor-container']");
  var gui = await addon.tab.waitForElement("#react-tabs-3");
  gui.addEventListener("keydown", handleKeyDown);
  gui.addEventListener("focusin", userStartedTyping);
  gui.addEventListener("focusout", userStoppedTyping);

I also wondered if there'd be an advantage to using document to not interrupt the workflow (when you're toggling between your game and costume editing). For example, if you eye dropped something on the game view or stage, your costume shortcuts would still work.

@misaunde
misaunde force-pushed the costume-editor-shortcuts branch from 4d1d62a to aeac759 Compare December 16, 2024 04:36
@misaunde
misaunde force-pushed the costume-editor-shortcuts branch from aeac759 to 2d2350b Compare December 16, 2024 04:39
@Samq64

Samq64 commented Dec 16, 2024

Copy link
Copy Markdown
Member

TurboWarp has this with the only differences being the key indicators are in the tooltips and the line tool is selected with L instead of D.

@WorldLanguages

Copy link
Copy Markdown
Member

TurboWarp has this with the only differences being the key indicators are in the tooltips and the line tool is selected with L instead of D.

Ideally we should match TurboWarp I think

@misaunde

misaunde commented Dec 17, 2024

Copy link
Copy Markdown
Contributor Author

Updated to match TurboWarp:

turbowarp.demo.2.mp4
scratch.demo.2.mp4

Tested in Chrome and Firefox.

Since scratch is for kids, I liked the idea of making the shortcuts obvious, but, agreed, it makes the most sense to match TurboWarp. Thanks for pointing out their implementation (now I'm a little confused why Scratch doesn't already have this feature 🤔)

@misaunde
misaunde force-pushed the costume-editor-shortcuts branch 2 times, most recently from 1246a4e to 109e4ca Compare December 17, 2024 05:48
@misaunde
misaunde force-pushed the costume-editor-shortcuts branch from 109e4ca to 6f0614a Compare December 17, 2024 05:52

@DNin01 DNin01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice work!

Comment thread addons/addons.json Outdated
Comment thread addons/costume-editor-shortcuts/addon.json
@misaunde
misaunde force-pushed the costume-editor-shortcuts branch from c54e149 to c53875e Compare December 18, 2024 01:21
@Samq64 Samq64 added status: needs review PR needs 1 more approval scope: from turbowarp Things to import from TurboWarp labels Dec 18, 2024
@misaunde misaunde mentioned this pull request Dec 18, 2024
Comment thread addons/costume-editor-shortcuts/userscript.js
@misaunde
misaunde force-pushed the costume-editor-shortcuts branch 2 times, most recently from d4b458a to 7fa2153 Compare January 2, 2025 22:34
@misaunde
misaunde force-pushed the costume-editor-shortcuts branch from 7fa2153 to e548e9a Compare January 2, 2025 22:50
Comment thread addons/costume-editor-shortcuts/addon.json Outdated
],
"versionAdded": "1.41.0",
"tags": ["editor", "costumeEditor"],
"enabledByDefault": false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"enabledByDefault": false
"enabledByDefault": true

This one will need some discussion.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm... This is a tough one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since it's a new addon, it seems safer to me to not enable it by default until it's proven itself stable, and then maybe enable by default in a future release?

@Samq64

Samq64 commented Jan 3, 2025

Copy link
Copy Markdown
Member

Another thing: If you still want the shortcut labels on the buttons it could be made a setting if you think it's worth it.

Co-authored-by: Samq64 <81489795+Samq64@users.noreply.github.com>
Comment thread addons/costume-editor-shortcuts/userscript.js Outdated
@misaunde

misaunde commented Jan 3, 2025

Copy link
Copy Markdown
Contributor Author

Another thing: If you still want the shortcut labels on the buttons it could be made a setting if you think it's worth it.

Yea, I wondered about that. I showed my son the PR at Christmas (he was also surprised to learn TurboWarp had this feature already) and he said: "I've been painting in Scratch for 3 years without shortcuts. I'm not going to hover over a button for 3 seconds to see what the new shortcut is." 😄

He's only a sample size of 1, but I kinda want to see how he adopts it before making further changes.

@WorldLanguages WorldLanguages left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's good enough now

Potential things to do next (or maybe never!)

  • Adding a setting to always show the keyboard shortcuts on the screen would be interesting
  • dynamicEnable & dynamicDisable (includes adding support for the case where the addon is toggled on while the user is already at the costume editor)
  • It's likely other addons also have their own checks for isUserTyping, and we're not reusing that code. It's also possible we're missing some edge cases as there's multiple places the user can be typing (e.g. Sign Up flow while costume editor open on the background)

@WorldLanguages WorldLanguages removed the status: needs review PR needs 1 more approval label Jan 3, 2025
@WorldLanguages
WorldLanguages merged commit 84c114a into ScratchAddons:master Jan 6, 2025
@DNin01

DNin01 commented Jan 27, 2025

Copy link
Copy Markdown
Member

Do you know if we addressed bitmap mode here? (I didn't check that yet)

@WorldLanguages

Copy link
Copy Markdown
Member

Yes, bitmap mode is considered

@WorldLanguages

Copy link
Copy Markdown
Member

Suggestion: #8335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new addon Related to new addons to this extension. `scope: addon` should still be added. scope: addon Related to one or multiple addons scope: from turbowarp Things to import from TurboWarp type: enhancement New feature for the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hotkey to change text in paint editor.

5 participants