Upgrade Blockly to v13.0.0-beta.5#11321
Conversation
Bump from 12.3.1 and drop the @blockly/keyboard-navigation plugin (now built into Blockly core). User-facing changes: - Keyboard navigation and screen-reader support are always enabled. Removed the accessible-blocks preference and its toggle UI. The heavier focus outlines for toolbox/workspace are enabled when using a meaningful keyboard navigation (previously matched setting). - Aligned pxt's cut/copy/paste with copy/cut toasts and the cut audio cue. - Variable creation focuses the new variable in the flyout (v13 behaviour); manual focus restore to the button removed. - Audio cues when keyboard navigation reaches limits and when a block is dropped on the workspace from move mode or drag. - Context menu changes were intentionally not ported from the keyboard navigation plugin. We could consider adding Cut alongside Copy/Paste, Copy/Paste are also not Blockly default. - Mac uses Cmd not Option as the unconstrained movement modifier. - Left and right arrow keys no longer wrap based on screen reader user feedback - Added an "end" position at the end of statement inputs. This can be useful for block insertion but is mainly motivated by helping screen reader users understand hierarchy in a way that's closer to text-based coding and more usable than a full tree structure. API: - Custom drag strategy collapsed from a substantial copy of Blockly's internals thin subclass due as the base is now exported. - Event handler shapes, navigator/focus manager flows, bubble interfaces and widget-div positioning all updated to match v13. - Shortcut formatting is now a tracked port of Blockly's internal helper (not on the public API). Workarounds for regressions in pxt customisations: - Preserve duplicate-shadow-on-drag (e.g. melody). Patched at the gesture and drag-strategy level. Previously this only worked because of an odd behaviour in Blockly where the shadow was briefly focused (you can actually see this in live MakeCode when clicking a block). - Snap dragged blocks back to the pointer at the snap-radius boundary (see RaspberryPiFoundation/blockly#9898). - Re-stack the connection-preview indicator over the highlight path. CSS: - Blockly v13 injects its CSS via adoptedStyleSheets, flipping the cascade. We've generally added body to selectors to address this. - The cascade change is currently under investigation to see if we can do better here in Blockly. - Related, adoptedStyleSheets requires Safari 16, last year we agreed 15, Blockly are up for a fix that's backwards compatible (will help with some iPads that still get 15.x security fixes) and micro:bit are taking a look here. Translation: - New aria labels, move-mode announcements, and screen-reader-mode toggle inherited from Blockly v13. Known regressions vs Blockly 12 + keyboard nav plugin: - Glitch when dragging affecting mouse users - RaspberryPiFoundation/blockly#9898 (Should be easy fix) - Initial block placement isn't intelligent - RaspberryPiFoundation/blockly#9878 (Blockly plan to fix) - Focused inputs not always respected for keyboard block insert - RaspberryPiFoundation/blockly#9899 (Newly raised) Follow ups will be needed for better screen reader integration and to update the keyboard controls help. Closes microsoft/pxt-microbit#6870
|
@riknoll FYI, we'll continue to test this and update it with Blockly beta releases. I'm away next week so @microbit-robert will pick up as needed. |
Blockly v13 injects its CSS via adoptedStyleSheets, applied after the
document stylesheets per spec, so its .blocklyDragging>.blocklyPath
{ fill-opacity: .8 } now wins the same-specificity tie against PXT's
{ fill-opacity: 0.7 } where in v12 source order favoured PXT. Bump
PXT's selector specificity with a body prefix, matching the pattern
used for the other v13-adopted-stylesheet overrides.
Drop the keyboard-mode line hide so the connection line shows during keyboard moves as well as mouse drags. Refresh the preview after the in-flight render queue drains rather than on the next animation frame. base.startDrag nudges the block by BLOCK_CONNECTION_OFFSET after firing the preview, and disconnecting from a parent can queue a static-block re-render that shifts the connection offsets — both leave the synchronous preview pass stale. Re-apply the indicator translate() transforms in the refresh pass so the dots track the post-render connection offsets rather than the pre-render positions baked in at creation time.
This had a weird first time behaviour where it just called setActive so didn't move to the flyout.
|
Discussed with @abchatra on today's dev sync. @riknoll, we think this is potentially mergeable (with future PRs upgrading to later betas/v13.0.0). But understandable to wait for the fixes to the regressions noted depending on your view of the impact. The next Blockly release will likely tweak strings too. Either way @riknoll we'd really appreciate some testing and review of this PR. |
| function initAccessibilityMessages() { | ||
| // Translatable overrides for Blockly's built-in keyboard-navigation strings. | ||
| // Excludes text used only in the shortcut dialog that we don't use. | ||
| Object.assign(Blockly.Msg, { |
There was a problem hiding this comment.
this looks good, but i wonder if we should provide some context in some of these strings for the benefit of translators. a lot of these are probably safe, but several of these have ambiguous definitions that the translators might not pick up on (e.g. "Checked" could mean "Evaluated", "text" could mean a few things, etc.)
if you haven't seen it before, the way we do this is by putting something like {id:checkbox} at the start of the string. everything in curlies gets stripped out by lf()
There was a problem hiding this comment.
I've tried this in 4231c8c. It feels quite a case-by-case judgement call so please do give feedback or just tweak them.
| socketbridge.tryInit(); | ||
| electron.initElectron(theEditor); | ||
| pxt.tickEvent( | ||
| "accessibilty.accessibleBlocksEnabledForSession", |
| * it to select the first category and clear the selection. | ||
| */ | ||
| const that = this; | ||
| Blockly.FlyoutNavigator.prototype.getOutNode = function(_node?: Blockly.IFocusableNode | null, _bypassAdjustments = false) { |
There was a problem hiding this comment.
just occurred to me that we should really be adding asserts for all the functions we monkey patch to ensure they exist. we've had trouble in the past when we monkey patch something that gets renamed in blockly core.
i wish we could keep this in pxtblocks/monkeyPatches, but i don't think there's a clean way to do it in this case.
There was a problem hiding this comment.
Done in 75b37e1, but I've not extended this to other places in pxtblocks as that would be a large diff.
I started with a "patchMethod" function but the diff/churn was quite high and type safety could only be maintained with faffy generics so seems easier to just assert before a simple assignment. I suggest any further assertions are tackled separately.
| return String.fromCharCode(keyCode); | ||
| } | ||
|
|
||
| const keyNames: Record<number, string> = { |
There was a problem hiding this comment.
let's add {keyboard} or something like that for the strings here to give the translators some context
There was a problem hiding this comment.
Reflecting on this, we should probably change these back to Blockly.Msg (aligning with Blockly internals this is copied from) and just address this in the initAccessibilityMessages above. Then the diff from Blockly's code stays small and we get the context in one place only.
There was a problem hiding this comment.
Done as part of I've tried this in 4231c8c which reverts these to Blockly.Msg and adds the context in initAccessibilityMessages.
| return chosen.map((shortcut) => | ||
| shortcut | ||
| .map((maybeNumeric) => | ||
| Number.isFinite(+maybeNumeric) |
There was a problem hiding this comment.
We could do this (I prefer it!), but this is a direct copy of Blockly internals (the change is us aligning it with the new Blockly core version rather than the keyboard nav version). As there's no behaviour change for the inputs used here I'm not sure a nit like this is worth the additional delta.
There are lots more to do in pxtblocks but limiting scope of this change intentionally.
|
Spotted keyboard controls help is missing shortcuts, likely due to id changes. We should fix up those here but do a separate PR for new help changes. |
Read shortcut key labels from Blockly.Msg in shortcut_formatting.ts so
those translations come from initAccessibilityMessages instead of
duplicating lf() calls and to keep the local copy close to upstream
Blockly's shortcut_formatting.ts.
Annotate the ambiguous accessibility messages with {id:...} translator
hints (keyboard symbol, block state, block role, block position, field
type, checkbox, dropdown, speech bubble) for strings whose meaning
isn't clear from the source word alone (e.g. "Checked", "text",
"command").
Use {id:keyboard symbol} for keys as is done elsewhere. Align a couple in the
keyboard controls help with this approach.
The local ShortcutNames enum was inherited from the blockly-keyboard-experiment plugin and was out of date now those shortcuts have moved into Blockly core. Several entries (TOOLBOX, CREATE_WS_CURSOR, EDIT_OR_CONFIRM, COPY, CUT, PASTE) had stale string values and were silently returning no shortcuts in the help dialog. Replace the enum with a single LIST_SHORTCUTS_SHORTCUT constant and Blockly.ShortcutItems.names. In blocks.tsx, the cleanup-workspace re-registration now keeps Blockly's CLEANUP name.
Fixed in 69d3e46. |
There are others to add in a separate PR but this is the only incorrect entry.
Demo deployment: https://blockly-v13-upgrade.review-pxt.pages.dev/#editor
Bump from 12.3.1 and drop the @blockly/keyboard-navigation plugin (now built into Blockly core).
User-facing changes:
API:
Workarounds for regressions in pxt customisations:
CSS:
Translation:
Known regressions vs Blockly 12 + keyboard nav plugin:
Follow ups will be needed for better screen reader integration and to update the keyboard controls help.
Closes microsoft/pxt-microbit#6870