Skip to content

feat: i shortcut on workspace gives overview#9677

Open
mikeharv wants to merge 3 commits intoRaspberryPiFoundation:v13from
mikeharv:i-shortcut
Open

feat: i shortcut on workspace gives overview#9677
mikeharv wants to merge 3 commits intoRaspberryPiFoundation:v13from
mikeharv:i-shortcut

Conversation

@mikeharv
Copy link
Copy Markdown
Contributor

@mikeharv mikeharv commented Apr 2, 2026

The basics

The details

Resolves

Fixes #9622

Proposed Changes

This adds a new 'i' shortcut for workspaces. If the workspace is focused, the shortcut will trigger an announcement of the current total of block stacks and workspace comments. New translation strings were added to facilitate communicating the number of stacks and comments in a grammatically accurate way. The new strings were intentionally phrased to front-load the most important information (ie. actual quantity) over contextual description (ie. "on the workspace").
See tests for sample announcement strings created using the new messages.

Reason for Changes

From linked issue:

when focused on workspace, i shortcut should tell how many stacks of blocks, how many comments, etc are on the workspace. stacks are more useful than total block count.

Test Coverage

The main new test performs a round trip through the different permutations of messages. The other test checks that the precondition logic works as expected (the workspace must be focused).

Documentation

N/A

Additional Information

I expect we'll want to make sure this new shortcut gets added to the keyboard navigation help menu/shortcuts list once that has been ported over.

@mikeharv mikeharv requested a review from a team as a code owner April 2, 2026 14:46
@mikeharv mikeharv requested a review from BenHenning April 2, 2026 14:46
@github-actions github-actions bot added the PR: feature Adds a feature label Apr 2, 2026
Copy link
Copy Markdown
Collaborator

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @mikeharv! Took a first pass and has some comments, PTAL.

Note that I'm not requesting changes in case you want to switch to a different reviewer since I'm actually out until next Wednesday.


registerDefaultShortcuts();
registerKeyboardNavigationShortcuts();
registerScreenReaderShortcuts();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we actually want to keep this out, for now. The idea is we'll use an optional plugin to enable these based on user prompting.

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.

Ah, okay. With this shortcut, I was thinking it reasonable to always include since it won't really do much if you're using a screen reader.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is quite true. You shouldn't need a plugin just to enable a feature that's already in core.

We haven't really fleshed out exactly what will be enabled by default yet, but I think everything that doesn't impact the experience for non-screenreader users should be enabled by default, and then a select few things are added via a settings toggle (the sounds as you navigate the workspace, disable looping are the ones I can think of). That way a screenreader user who doesn't discover the additional features still gets as much utility as possible.

That being said feel free to keep this method and not call it yet, we can call it or not later.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The problem I see is that this still uses up the keyboard binding, and I’m assuming makes populating the keyboard navigation help menu a bit trickier since it presumably needs to account for only showing these when the “screen reader mode” thing is enabled. My thinking was gating the actual registration of the shortcuts makes both cases easier, but perhaps you’re thinking of this differently?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am writing a document now about what we'll do about default options, so we don't need to debate it in this PR :) I'll share it with you when ready.

Comment on lines +424 to +428
"ARIA_WORKSPACE_BLOCKS_MANY": "%1 stacks of blocks%2 in workspace.",
"ARIA_WORKSPACE_BLOCKS_ONE": "One stack of blocks%2 in workspace.",
"ARIA_WORKSPACE_BLOCKS_ZERO": "No blocks%2 in workspace.",
"ARIA_WORKSPACE_COMMENTS_MANY": " and %1 comments",
"ARIA_WORKSPACE_COMMENTS_ONE": " and one comment"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm finding this a bit hard to grok, and I suspect a translator might as well. Perhaps we could be slightly repetitive to simplify things. What if we avoided the double concatenation and had four different variables for the label:

  • "%1 stacks of blocks and %2 comments in workspace."
  • "One stack of blocks and %2 comments in workspace."
  • "%1 stacks of blocks and one comment in workspace."
  • "One stack of blocks and one comment in workspace."

Separately, I'm not sure about the Msg name here. These aren't actual ARIA labels or properties, they're just strings that happen to be used as a status message to trick the screen reader into announcing them I do recognize that we will want to standardize on this, though, so perhaps @maribethb might have a suggestion. One thought might be to do something like: WORKSPACE_MANY_BLOCKS_AND_MANY_COMMENTS_SCREEN_READER_ANNOUNCEMENT_LABEL but I'm not sure sure just how specific we want to get with these.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, I think going with the 4 separate messages is more straightforward for translators. I might add a 5th one which is "Workspace is empty" for that case. You mentioned you wrestled with a couple different options, what made you not choose this one?

Agree I wouldn't use Aria in the name, but I think we can come up with a shorter name than Ben's suggestion. The fact that it's primarily a label for screenreader announcement can be explained in the message comment, the same way we do for e.g. "Tooltip" and "Dropdown menu option". So I would suggest something like WORKSPACE_CONTENTS_MANY_BLOCKS_MANY_COMMENTS etc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, we discussed a bit more in real time and one of the things that the original proposal gives us that the 4 separate comments doesn't is the ability to leave off information about comments entirely. We actually don't include workspace comments by default in Blockly, as you have to enable the context menu item that lets you add them separately. Some products don't use them at all (e.g. code.org). So it would be confusing for those users to always be told that there are no workspace comments when they aren't even aware that those might exist.

So I think if we add more examples in the message description that the original design is workable. Also, Ben, had you been able to read the message descriptions when you commented here? Since you commented on en.js you don't see the descriptions in that file, but a translator would see them in the console.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I admittedly hadn’t looked at the descriptions since I reviewed this a bit quickly and figured they would change if we changed the format itself.

I agree with the benefit of omitting comments though the current implementation doesn’t make this distinction. We could perhaps just not include the word ‘comments’ if there are zero to account for that case and still benefit from having the four simpler messages. Could that work?

Pedagogically I don’t think specifying 0 comments is actually all that useful since comments are very much a secondary or tertiary user flow/concept.

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.

Pedagogically I don’t think specifying 0 comments is actually all that useful since comments are very much a secondary or tertiary user flow/concept.

Agreed. That's actually the main motivator of the current implementation here.

The current implementation skips the phrase and n comments entirely if there aren't any. Whether there are comments or not, we still need to be able to report that there are zero/one/many stacks. There could also be zero/one/many stacks. (Note that it's advantageous to avoid grouping 'zero' with the plural version, because not all languages work that way.)
I believe these are the permutations we'd need to support, assuming we're aligned with dropping the comments part entirely when there are zero:

  • [0, 0] "Workspace is empty" (or "No blocks on workspace")
  • [1, 0] "1 stack of blocks on workspace"
  • [2+,0] "2 stack of blocks on workspace"
  • [0, 1] "No blocks and 1 comment on workspace"
  • [1, 1] 1 stack of block and 1 comment on workspace"
  • [2+,1] "2 stacks of blocks and 1 comment on workspace"
  • [0, 2+] "No blocks and 2 comments on workspace"
  • [1, 2+] "1 stack of blocks and 2 comments on workspace"
  • [2+,2+] "2 stacks of blocks and 2 comments on workspace"

It's not clear to me how we could do this with four simpler messages.

Copy link
Copy Markdown
Contributor

@maribethb maribethb Apr 3, 2026

Choose a reason for hiding this comment

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

The current implementation does make this distinction, as you can leave off the "and x comments" message.

We could perhaps just not include the word ‘comments’ if there are zero to account for that case and still benefit from having the four simpler messages. Could that work?

That's what the current implementation does, but I'm not sure how you could do it with the messages you suggested earlier. Can you clarify?

edit: sorry did not see Mike's comment above when I wrote this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah you’re both right that this can’t be done easier. Well I’m certainly not going to be insistent on it being simpler since what we’ll be doing for block labels is going to be much more complex than this and translators hopefully will be able to figure it out. Splitting up seems reasonable.

@BenHenning BenHenning assigned mikeharv and unassigned BenHenning Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: feature Adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants