Skip to content

fix: Make zoom to fit plugin keyboard and screenreader accessible#2728

Open
gonfunko wants to merge 1 commit into
v13from
zoom-to-fit
Open

fix: Make zoom to fit plugin keyboard and screenreader accessible#2728
gonfunko wants to merge 1 commit into
v13from
zoom-to-fit

Conversation

@gonfunko

@gonfunko gonfunko commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

The basics

The details

Proposed Changes

This PR fixes #2712 by making the zoom to fit control keyboard navigable and screenreader accessible following a similar pattern to the built-in zoom controls. This depends on a new release of core that adds Capability.FOCUSABLE. With these changes, the control can be tabbed to, enter/space will activate it, it has a screenreader label and role of button, and focusing it will draw a highlight ring and darken the icon in the same manner as hovering over it.

@gonfunko gonfunko requested a review from a team as a code owner June 24, 2026 16:25
@gonfunko gonfunko requested review from mikeharv and removed request for a team June 24, 2026 16:25

@mikeharv mikeharv left a comment

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.

Looks good. My questions do not block approval.

Blockly.utils.aria.setState(
this.svgGroup,
Blockly.utils.aria.State.LABEL,
'Zoom to fit',

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.

Translation?

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.

Yes, but I think we're better doing a pass across all of samples since the translation situation overall is pretty poor. This will become much less painful once it's a monorepo as well.

this.svgGroup,
);

const image = Blockly.utils.dom.createSvgElement(

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.

Do we need to hide this element from the accessibility tree?

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.

Experimentally it seemed fine, the screenreader wasn't attempting to read it

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.

2 participants