From 50a7c7cd2a622fed10895f5e95a9471cf8d338a6 Mon Sep 17 00:00:00 2001 From: Laura Sach <5183697+lawsie@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:16:13 +0100 Subject: [PATCH 1/4] Shorten comments --- main/blocklyinit.js | 31 ++++++------------------------- scripts/run-api-tests.mjs | 1 - ui/colourpicker.js | 1 - 3 files changed, 6 insertions(+), 27 deletions(-) diff --git a/main/blocklyinit.js b/main/blocklyinit.js index 3b0afb0d..ea6ddd05 100644 --- a/main/blocklyinit.js +++ b/main/blocklyinit.js @@ -1489,14 +1489,8 @@ function installShadowNavigationPatch(ws) { return skipBlock(origPrev(node)); }; - // The built-in DISCONNECT shortcut (X key) checks that the focused node is - // a Block instance, which fails when focus is on a skippable block's field - // (because we skip the block stop). Register an additional shortcut for the - // same key that fires only when a skippable field is focused. - // The built-in DISCONNECT (X), DUPLICATE (D), and DELETE shortcuts check - // that the focused node is a Block instance, which fails when focus is on a - // skippable block's field. Register additional shortcuts for the same keys - // that fire only when a skippable field is focused. + // Built-in DISCONNECT (X), DUPLICATE (D) and DELETE check the focused node is a Block, + // which fails on a skippable block's field — re-register the same keys for that case. const shortcutRegistry = Blockly.ShortcutRegistry.registry; const skippableFieldBlock = () => { @@ -1747,16 +1741,8 @@ export function createBlocklyWorkspace() { workspace = Blockly.inject('blocklyDiv', options); - // Stop trashcan flyout from covering the whole workspace on small screens when it has wide blocks in it - // The trashcan flyout is as wide as its widest deleted block, so a wide block - // can make it cover the whole workspace with nothing left to click to dismiss - // it. Cap its rendered width to the visible workspace minus a small gap. - // Blockly keeps the flyout right-aligned, so the right edge (and its vertical - // scrollbar) stay pinned to the workspace edge while the left edge can never - // cross the gap; wider blocks overflow to the right. Recomputed from live - // metrics on every position() call, so it tracks the panel resizer; the gap - // is capped to a fraction of the visible workspace so it stays sensible on - // narrow panels. + // Stop trashcan flyout from covering the whole workspace on small + // screens when it has wide blocks in it const trashcan = workspace.trashcan; const trashcanFlyout = trashcan?.flyout; if (trashcan && trashcanFlyout) { @@ -1768,13 +1754,8 @@ export function createBlocklyWorkspace() { return Math.min(this.width_, viewWidth - gap); }; - // The flyout is a separate, higher z-index SVG that would otherwise hide the - // trashcan icon. While the flyout is open, lift the icon into an overlay SVG - // that shares the workspace's coordinate space, so it stays in place but - // The flyout is a separate, higher z-index SVG that would otherwise hide the - // trashcan icon. While the flyout is open, lift the icon into an overlay SVG - // that shares the workspace's coordinate space, so it stays in place but - // renders on top; clicking the lifted icon then closes the flyout. + // Lift the trashcan icon into an overlay SVG while the flyout is open — the + // higher z-index flyout would otherwise hide it. const injectionDiv = workspace.getInjectionDiv(); let trashIcon = null; try { diff --git a/scripts/run-api-tests.mjs b/scripts/run-api-tests.mjs index e1ae98bd..b1d87745 100644 --- a/scripts/run-api-tests.mjs +++ b/scripts/run-api-tests.mjs @@ -15,7 +15,6 @@ const __dirname = path.dirname(__filename); */ // Available test suites (from tests/tests.html) -// Note: 228 tests total are registered, but most are tagged const AVAILABLE_SUITES = [ { id: '@notslow', diff --git a/ui/colourpicker.js b/ui/colourpicker.js index c78b481a..564e6fc4 100644 --- a/ui/colourpicker.js +++ b/ui/colourpicker.js @@ -1747,7 +1747,6 @@ class CustomColorPicker { return brightness > 128 ? '#000000' : '#ffffff'; } - // Keep the indicator fully inside the wheel (accounts for its own size + outline) // Keep the indicator fully inside the ring (accounts for its size & border) _indicatorPad() { const el = this.colorWheelIndicator; From aebc62594321f46576725ed204baadef0e786026 Mon Sep 17 00:00:00 2001 From: Laura Sach <5183697+lawsie@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:40:10 +0100 Subject: [PATCH 2/4] Blocklyinit comments --- accessibility/accessibility.js | 5 - api/animate.js | 7 +- main/blocklyinit.js | 202 ++++++++++----------------------- 3 files changed, 59 insertions(+), 155 deletions(-) diff --git a/accessibility/accessibility.js b/accessibility/accessibility.js index 6fdbb9e1..29aa39a7 100644 --- a/accessibility/accessibility.js +++ b/accessibility/accessibility.js @@ -21,9 +21,6 @@ let sayAnnounceSeq = 0; // Helps avoid repeating the same click announcement too many times in a row let lastInteractionKey = ''; let lastInteractionTime = 0; - -// Two persistent assertive regions that alternate so each announce writes to whichever -// was previously empty — guarantees the reader sees a genuine "" → text change every time. let _interactRegions = null; let _interactRegionIdx = 0; @@ -121,8 +118,6 @@ export function announce(message, options = {}) { lastAnnouncedAt = now; if (noDedup) { - // Alternate between two pre-registered assertive regions. Each press writes to the - // one that was previously empty, so the reader always sees a genuine "" → text change. // Freshly-created assertive elements have a registration delay; pre-registered ones don't. if (!_interactRegions) { const root = createA11yRoot(); diff --git a/api/animate.js b/api/animate.js index 65cd5af7..281c2cc0 100644 --- a/api/animate.js +++ b/api/animate.js @@ -1443,9 +1443,7 @@ export const flockAnimate = { previousGroup !== retargetedGroup && mesh._animationEverPlayed; if (shouldBlend) { - // Always stop the outgoing animation immediately and freeze its pose as a - // snapshot. Letting a live animation continue to drive bones (e.g. - // oscillating antennae) during the weight fade causes jitter. + // Always stop the outgoing animation immediately and freeze its pose if (outgoingGroup) { outgoingGroup.stop(); } @@ -1780,9 +1778,6 @@ export const flockAnimate = { previousGroup !== targetAnimationGroup && skeletonMesh?._animationEverPlayed; if (shouldBlend) { - // Always stop the outgoing animation immediately and freeze its pose as a - // snapshot. Letting a live animation continue to drive bones (e.g. - // oscillating antennae) during the weight fade causes jitter. if (outgoingGroup) { outgoingGroup.stop(); } diff --git a/main/blocklyinit.js b/main/blocklyinit.js index ea6ddd05..75f9b17c 100644 --- a/main/blocklyinit.js +++ b/main/blocklyinit.js @@ -45,9 +45,7 @@ import { } from '../ui/blocklyutil.js'; import { toolbox as toolboxDef } from '../toolbox.js'; -// Persist locked blocks as part of the workspace serialization. Lower priority -// than the built-in 'blocks' serializer (20) so the target blocks already exist -// when we re-apply the lock on load. +// Priority 0 — below the 'blocks' serializer's 20, so blocks exist before locks are re-applied. if (!Blockly.serialization.registry.getClass?.('flockLock')) { Blockly.serialization.registry.register('flockLock', { priority: 0, @@ -59,10 +57,8 @@ if (!Blockly.serialization.registry.getClass?.('flockLock')) { return ids.length ? ids : undefined; }, load(state, ws) { - // Apply lock state per saved block (every locked block was recorded), not - // by re-cascading from a root — the tree may have gained blocks (e.g. a - // duplicate connected after a locked block) since it was locked, and those - // must not be swept into the lock on reload. + // Apply per block rather than re-cascading from a root: the tree may have gained + // blocks since locking (e.g. a duplicate), and those must not be swept into the lock. for (const id of state || []) { const b = ws.getBlockById(id); if (b) applyBlockLockState(b, true); @@ -104,10 +100,8 @@ if (!Blockly.serialization.registry.getClass?.('flockLock')) { }; } -// After jsonInit builds a block's inputs, give its value/statement inputs ARIA -// labels so screen readers announce each input's context on focus. A block can -// supply an `ariaLabels` map (keyed by input name) in its definition to -// override or suppress individual labels; the key is ignored by jsonInit. +// Blocks may carry an `ariaLabels` map (keyed by input name) in their jsonInit definition to +// override or suppress per-input labels; jsonInit itself ignores the key. { const originalJsonInit = Blockly.Block.prototype.jsonInit; Blockly.Block.prototype.jsonInit = function (json) { @@ -116,18 +110,10 @@ if (!Blockly.serialization.registry.getClass?.('flockLock')) { }; } -// A "simple reporter" (e.g. a number plugged into scale's X slot) announces -// only its field's value, with no per-input context. We prepend the parent -// input's ARIA label ("x") to the field's announced element so navigating onto -// it reads "x, number: 0". This is done in recomputeAriaContext (which sets the -// element's aria-label), NOT computeAriaLabel: the parent block composes its own -// readout from each child's computeAriaLabel, so prefixing there would make the -// block say the slot label twice (once as its field-row label, once via the -// child). The element-only prefix keeps the block readout clean. +// Prefix a simple reporter's field with its parent slot label ("x, number: 0") in +// recomputeAriaContext, NOT computeAriaLabel — the parent would then say the slot twice. { - // The slot label and whether to set one at all (sibling-disambiguation, - // overrides) are decided in applyInputAriaLabels; here we just surface - // whatever provider the parent input carries. + // Which label, and whether to set one at all, is decided in applyInputAriaLabels. const parentSlotLabel = (field) => { const block = field.getSourceBlock?.(); if (!block || !block.isSimpleReporter?.() || block.getFullBlockField?.() !== field) { @@ -137,11 +123,9 @@ if (!Blockly.serialization.registry.getClass?.('flockLock')) { block.outputConnection?.targetConnection ?? block.previousConnection?.targetConnection; return conn?.getParentInput?.()?.getAriaLabelText?.() ?? null; }; - // Several field types define their own recomputeAriaContext, each calling - // super: Field, FieldInput (base of FieldTextInput/FieldNumber; not exported, - // so reached via the prototype chain), FieldDropdown (→ FieldVariable) and - // FieldCheckbox. Wrap every prototype that owns the method; a per-instance - // re-entrancy guard ensures the slot is prepended once, to the final label. + // Wrap every prototype owning recomputeAriaContext: Field, FieldInput (base of + // FieldTextInput/FieldNumber, not exported — hence the prototype-chain hop), FieldDropdown + // (→ FieldVariable), FieldCheckbox. They chain to super, hence the re-entrancy guard below. const textInputProto = Blockly.FieldTextInput?.prototype; const candidateProtos = [ Blockly.Field?.prototype, @@ -165,9 +149,8 @@ if (!Blockly.serialization.registry.getClass?.('flockLock')) { const el = this.getFocusableElement?.(); const current = el?.getAttribute?.('aria-label'); const prefix = `${slot}, `; - // v13.1.0 added native full-block field label prefixing, which can - // already have set this prefix before we get here — guard so we - // don't double it up ("hair, hair, color: Red"). + // v13.1.0 may already have applied this prefix natively — guard against + // doubling it ("hair, hair, color: Red"). if (el && current && !current.startsWith(prefix)) { el.setAttribute('aria-label', `${prefix}${current}`); } @@ -392,16 +375,15 @@ function initializeIfClauseConnectionChecker(workspace) { return true; } + // Reject while dragging (visual feedback); otherwise allow the connection and let + // validateIfClausePositions disable the block in-place (healing, undo/redo, field change). if (connectingToNext) { // Moving block is connecting AFTER target if (targetIsIfClause) { const targetMode = targetBlock.getFieldValue('MODE'); - // Rule 1: Nothing can connect after ELSE. - // During drag-and-drop reject to give visual feedback. - // During healing / field-changes (not dragging) allow the connection; - // validateIfClausePositions will disable the block in-place. + // Rule 1: nothing can connect after ELSE. if (targetMode === MODE.ELSE) { if (workspace.isDragging()) return false; } @@ -411,18 +393,13 @@ function initializeIfClauseConnectionChecker(workspace) { return false; } - // Rule 3: ELSE cannot be inserted in middle of chain (drag only). - // When not dragging (e.g. a MODE field change), keep the connection - // and let validateIfClausePositions disable the block in-place. + // Rule 3: ELSE cannot be inserted mid-chain. const targetHasNext = realNext(targetBlock); if (targetHasNext && targetHasNext.type === 'if_clause' && movingMode === MODE.ELSE) { if (workspace.isDragging()) return false; } } else { - // Target is NOT if_clause. - // During drag-and-drop reject to give visual feedback. - // During healing / undo-redo (not dragging) allow the connection; - // validateIfClausePositions will disable the block in-place. + // Target is NOT if_clause: ELSEIF/ELSE need an if_clause predecessor. if (movingMode === MODE.ELSEIF || movingMode === MODE.ELSE) { if (workspace.isDragging()) return false; } @@ -475,9 +452,7 @@ function initializeIfClauseConnectionChecker(workspace) { // Otherwise it's fine - connecting at the end of the chain return true; } else { - // Non-if_clause connecting before if_clause - // Only allow before IF (which can start a new chain) - // Don't allow before ELSEIF or ELSE + // Non-if_clause may only go before IF, which can start a new chain. if (targetMode === MODE.ELSEIF || targetMode === MODE.ELSE) { return false; } @@ -491,9 +466,7 @@ function initializeIfClauseConnectionChecker(workspace) { // connected but in an invalid position (e.g. ELSEIF after a regular block). const INVALID_IF_CLAUSE_REASON = 'INVALID_IF_CLAUSE_POSITION'; - // Scan all if_clause blocks and disable/enable them based on whether their - // predecessor is a valid if_clause. Runs with events disabled so the - // enable/disable state is derived (not recorded in the undo stack). + // Events disabled so the derived enable/disable state never enters the undo stack. function validateIfClausePositions() { Blockly.Events.disable(); try { @@ -528,9 +501,7 @@ function initializeIfClauseConnectionChecker(workspace) { } } - // Re-validate after any structural change so that if_clause blocks that - // land in an invalid position are disabled immediately, and those that - // become valid again are re-enabled. + // Re-validate on structural change so invalid clauses disable and valid ones re-enable. workspace.addChangeListener(function (event) { if ( !event.isUiEvent && @@ -840,10 +811,8 @@ export function initializeWorkspace() { ); }; - // Desktop flyout search is handled by Blockly's native matchBlocks via the - // blockSearcher.blockTypesMatching override in overrideSearchPlugin. The - // mobile overlay below reuses that same searcher (blockTypesMatching) so - // desktop and mobile stay consistent. + // The mobile overlay reuses blockSearcher.blockTypesMatching (overridden in + // overrideSearchPlugin) so it stays consistent with the desktop flyout. // Build overlay bar const overlay = document.createElement('div'); @@ -1071,11 +1040,8 @@ export function initializeWorkspace() { if (resultsPanel.isConnected) { updateResults(); } else { - // The toolbox-search plugin runs matchBlocks() from the field's - // keydown handler, which fires before the pressed key reaches - // input.value — so the flyout query always lagged one character - // behind (typing "map" searched "ma" and showed nothing until a - // further edit). Re-run the search here, when the value is current. + // The plugin calls matchBlocks() from keydown, before input.value updates, so the + // query lagged a character ("map" searched "ma"). Re-run it where the value is current. searchCategory?.matchBlocks?.(); } }); @@ -1327,9 +1293,7 @@ export function initializeWorkspace() { const leftMargin = 10; const buffer = 5; - // 1. HORIZONTAL LOGIC: - // Only move X if the block's left edge is hidden or too far right. - // We ignore the specific row's internal X offset to keep it stable. + // Horizontal: move X only if the left edge is hidden; ignore row X offset for stability. let finalScrollX = this.scrollX; const isHorizontallyVisible = currentBlockX >= leftMargin - buffer && currentBlockX + blockWidth <= viewportWidth; @@ -1338,8 +1302,7 @@ export function initializeWorkspace() { finalScrollX = -blockXY.x * scale + leftMargin; } - // 2. VERTICAL LOGIC: - // Strict check for the specific row (y + yOffset) + // Vertical: strict check for the specific row (y + yOffset). const currentRowY = currentBlockY + yOffset * scale; const isRowVisible = currentRowY >= searchBarHeight + buffer && @@ -1350,7 +1313,6 @@ export function initializeWorkspace() { finalScrollY = -(blockXY.y + yOffset) * scale + 50; } - // 3. EXECUTION: if (finalScrollX === this.scrollX && finalScrollY === this.scrollY) { return; } @@ -1369,16 +1331,9 @@ export function initializeWorkspace() { return workspace; } -// Patch the workspace Navigator so keyboard navigation skips redundant stops -// on value blocks whose only interactive content is a text-input field. -// -// Applies to two cases: -// Shadow blocks — e.g. the " " text block inside print_text's TEXT input. -// Standalone text-input reporters — e.g. `text` and `colour_from_string`. -// -// In all cases: right-arrow and left-arrow skip the block entirely and land -// on the field (right) or the parent (left). Up/down navigate as if standing -// on the block itself. +// Patch the Navigator so keyboard navigation skips the redundant block stop on value blocks +// whose only interactive content is a text-input field — shadow (" " in print_text) and +// standalone (`text`, `colour_from_string`) alike. function installShadowNavigationPatch(ws) { const nav = ws.getNavigator?.(); if (!nav) return; @@ -1400,45 +1355,29 @@ function installShadowNavigationPatch(ws) { return null; }; - // A shadow value block whose primary field is *separately* navigable, which - // creates a redundant block+field double-stop during keyboard navigation - // (e.g. the " " `text` block in print_text's TEXT input). - // - // Simple reporters (single field + output, e.g. `math_number`) are excluded: - // Blockly already makes their full-block field non-navigable and treats the - // block itself as the single stop (Enter edits it directly), so there is no - // redundant stop to skip — and redirecting to their non-navigable field - // would break navigation. + // Simple reporters are excluded: Blockly already makes their full-block field + // non-navigable, so redirecting to it would break navigation. const isSkippableShadow = (node) => typeof node?.isShadow === 'function' && node.isShadow() && !!node.outputConnection && !(typeof node.isSimpleReporter === 'function' && node.isSimpleReporter()); - // A standalone (non-shadow) reporter block whose sole interactive content is - // a text-input field, e.g. `text` (" ") or `colour_from_string` (# hex). - // These create the same redundant block+field double-stop as skippable shadows. - // - // Variable/dropdown reporters are excluded because their primary field is a - // FieldDropdown/FieldVariable, not a FieldTextInput. - // Simple reporters are excluded for the same reason as isSkippableShadow. + // Variable/dropdown reporters are excluded (their primary field isn't a FieldTextInput), + // simple reporters for the same reason as isSkippableShadow. const isSkippableStandalone = (node) => !!node?.outputConnection && !node.isShadow?.() && !(typeof node.isSimpleReporter === 'function' && node.isSimpleReporter()) && getPrimaryEditableField(node) != null; - // If node is a skippable block (shadow or standalone), return its primary - // field instead. const skipBlock = (node) => { if (!isSkippableShadow(node) && !isSkippableStandalone(node)) return node; return getPrimaryEditableField(node) ?? node; }; - // The shortcut handler calls getInNode/getOutNode/getNextNode/getPreviousNode - // with no arguments, relying on the focused node. A skippable block's - // full-block field resolves back to its block via getFocusedNode(), so we - // read document.activeElement to recover the field that actually owns focus. + // getFocusedNode() resolves a full-block field back to its block, so recover the field + // that actually owns focus from document.activeElement. const getFocusedSkippableField = () => { const el = document.activeElement; if (!el?.id) return null; @@ -1459,16 +1398,14 @@ function installShadowNavigationPatch(ws) { const origNext = nav.getNextNode.bind(nav); const origPrev = nav.getPreviousNode.bind(nav); - // Right-arrow: if the target is a skippable block, land on its field instead - // of the redundant block stop. From a skippable field, pass the field - // explicitly so the traversal bubbles up to the next inline sibling. + // Right-arrow: land on the field, not the redundant block stop. From a skippable field, + // pass the field explicitly so traversal bubbles to the next inline sibling. nav.getInNode = function (node) { const field = getFocusedSkippableField(); return skipBlock(field ? origIn(field) : origIn(node)); }; - // Left-arrow: from a skippable block's field, go to the block's parent - // (skip the block itself in both the shadow and standalone cases). + // Left-arrow: from a skippable field, go to the block's parent, skipping the block. nav.getOutNode = function (node) { const field = getFocusedSkippableField(); if (field) return skipBlock(origOut(field.getSourceBlock())); @@ -1529,10 +1466,8 @@ function installShadowNavigationPatch(ws) { } ); - // Shared duplicate: copy the block and paste it, stripping lock state so a - // copy of a locked block comes out editable/movable. Always returns true so - // dispatch stops here (a falsy return would let another D-bound shortcut fire - // and paste a second time). + // Strips lock state so a copy of a locked block is editable. Returns true to stop + // dispatch — a falsy return lets another D-bound shortcut paste a second time. const duplicateViaClipboard = (ws, block) => { const copyData = block?.toCopyData?.(); if (!copyData) return false; @@ -1587,18 +1522,13 @@ function installShadowNavigationPatch(ws) { } ); - // Comment shortcuts. 'K' toggles the comment bubble open/closed (creating one - // and focusing it when none exists); 'Shift+K' deletes the comment. (N — the - // natural mnemonic for "note" — is already Blockly's next_stack nav key.) - // Comment has no built-in Blockly shortcut, so unlike X/D/Delete these must - // resolve the target themselves — from the focused block (scope.focusedNode), - // or a focused field's source block, falling back to the skippable-field - // resolver. + // K toggles the comment bubble, Shift+K deletes it (N, the natural mnemonic, is already + // Blockly's next_stack key). No built-in shortcut exists, so unlike X/D/Delete these + // resolve their own target. { const commentTargetBlock = (scope) => { const node = scope?.focusedNode; - // A focused block exposes getCommentText; a focused field exposes - // getSourceBlock and unwraps to the block that owns it. + // A focused block exposes getCommentText; a focused field unwraps via getSourceBlock. if (node) { if (typeof node.getCommentText === 'function') return node; if (typeof node.getSourceBlock === 'function') { @@ -1626,9 +1556,8 @@ function installShadowNavigationPatch(ws) { // triggered this lands in the comment editor we're about to focus. event?.preventDefault?.(); Blockly.Events.setGroup('comment_shortcut'); - // The undoable create runs synchronously inside toggleCommentBubble - // before it awaits, so it lands in this group; the bubble open/focus - // that follows is UI state and needn't be grouped. + // The undoable create runs synchronously before toggleCommentBubble awaits, so it + // lands in this group; the bubble open/focus that follows is UI state. toggleCommentBubble(block); Blockly.Events.setGroup(false); return true; @@ -1703,9 +1632,7 @@ export function createBlocklyWorkspace() { } } - // Accordion behaviour: navigating to a category collapses every other - // expandable category, keeping only the selected item and its ancestors - // open. + // Accordion behaviour: keep only the selected item and its ancestors expanded. collapseUnrelatedCategories_(selectedItem) { if (!selectedItem) return; const keepOpen = new Set(); @@ -1779,9 +1706,7 @@ export function createBlocklyWorkspace() { }); injectionDiv.appendChild(iconOverlay); - // pointer-events is inherited, so the overlay's `none` (which lets clicks - // pass through its empty area to the flyout) would also disable the icon. - // Force the icon itself back to clickable so it can close the flyout. + // pointer-events is inherited, so the overlay's `none` would disable the icon too. trashIcon.style.pointerEvents = 'auto'; // Clicking the lifted icon closes the flyout. The icon's own pointerup @@ -1792,10 +1717,8 @@ export function createBlocklyWorkspace() { if (Date.now() < suppressOpenUntil) return; originalOpenFlyout(); }; - // Move keyboard focus into the flyout the instant its blocks render. openFlyout() - // renders on a deferred timeout and fires TRASHCAN_OPEN before the blocks exist, - // so instead of polling, hook show() — the call that builds the blocks. Right - // after it, getTopBlocks() is populated. + // TRASHCAN_OPEN fires before the deferred render, so hook show() instead — it is the + // call that builds the blocks, and getTopBlocks() is populated right after it. const originalFlyoutShow = trashcanFlyout.show.bind(trashcanFlyout); trashcanFlyout.show = function (contents) { originalFlyoutShow(contents); @@ -2508,12 +2431,9 @@ export function overrideSearchPlugin(workspace) { } SearchCategory.prototype.initBlockSearcher = function () { - // Deliberately skip the plugin's default trigram indexing here: Flock - // searches its own rich index (workspace.flockSearchIndexedBlocks, built by - // buildSearchIndex) for both the desktop flyout and the mobile overlay, - // which also covers author keyword synonyms the trigram index lacks. Not - // building the trigram index avoids instantiating every block twice at - // startup. this.blockSearcher is already created by the plugin constructor. + // Deliberately skip the plugin's trigram indexing: Flock searches its own index + // (flockSearchIndexedBlocks), which also covers keyword synonyms, and skipping avoids + // instantiating every block twice at startup. const blockSearcher = this.blockSearcher; const rebuildSearchIndex = () => { @@ -2531,11 +2451,8 @@ export function overrideSearchPlugin(workspace) { this.blockSearcher.indexBlocks = rebuildSearchIndex; blockSearcher.indexedBlocks_ = workspace.flockSearchIndexedBlocks || null; - // Override only the search primitive: substring-match Flock's rich index - // (keyword synonyms, block labels, dropdown alt text, shadow defaults), - // relevance-sorted and de-duplicated by type. Blockly's native matchBlocks - // consumes this, so Blockly keeps handling flyout rendering, the screen - // reader announcement, and the empty / no-match labels. + // Override only the search primitive — Blockly's native matchBlocks consumes it, so + // flyout rendering, screen-reader announcements and no-match labels stay upstream. blockSearcher.blockTypesMatching = (rawQuery) => { const q = (rawQuery || '').toLowerCase().trim(); if (!q) return []; @@ -2784,11 +2701,8 @@ export function overrideSearchPlugin(workspace) { searchToolboxItem.initBlockSearcher(); } - // Keep Blockly's native matchBlocks: it calls our blockSearcher.blockTypesMatching - // override for results and retains ownership of flyout rendering and the - // screen-reader announcement. Wrap it thinly only to localise the placeholder - // labels the plugin hard-codes in English ("Type to search for blocks" when the - // query is empty, "No matching blocks found" when a query matches nothing). + // Wrap native matchBlocks only to localise the placeholder labels the plugin hard-codes + // in English ("Type to search for blocks", "No matching blocks found"). const nativeMatchBlocks = SearchCategory.prototype.matchBlocks; SearchCategory.prototype.matchBlocks = function () { nativeMatchBlocks.call(this); From 40149aaa16297f4b7793fcc08b6020bf1a585915 Mon Sep 17 00:00:00 2001 From: Laura Sach <5183697+lawsie@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:52:21 +0100 Subject: [PATCH 3/4] Flock.js comments --- flock.js | 85 ++++++++++++++++---------------------------------------- 1 file changed, 24 insertions(+), 61 deletions(-) diff --git a/flock.js b/flock.js index a597697e..c5404ce7 100644 --- a/flock.js +++ b/flock.js @@ -213,9 +213,8 @@ export const flock = { ) { key = 'recursion_too_deep'; } - // One fault can fire in a burst — a stack-overflow cascade surfaces several - // times, and a per-frame loop error repeats endlessly. Collapse identical - // reports within a short window. + // One fault can fire in a burst (stack-overflow cascade, per-frame loop error), so + // collapse identical reports within a short window. const now = Date.now(); const sig = `${key}\0${api ?? ''}\0${safe?.message ?? ''}`; if (sig === flock._lastErrorSig && now - flock._lastErrorAt < 1000) { @@ -232,11 +231,9 @@ export const flock = { // a broken listener must not break the swallow path } }, - // User code can throw any value, including an object whose name/message/stack - // getters throw or return non-strings. Read each once, defensively, into a - // plain Error so host handlers touch only safe primitives. (A getter that - // hangs rather than throws can't be defended against — that's a self-inflicted - // hang, like any infinite loop in user code.) + // User code can throw any value, including one whose name/message/stack getters throw or + // return non-strings. Read each once into a plain Error so host handlers see only safe + // primitives. A getter that hangs is undefendable — that's user code hanging itself. sanitizeError(error) { const read = (get) => { try { @@ -301,9 +298,7 @@ export const flock = { return errorContext; }, - // Prune disposed entries and auto-recycle the oldest live instance - // when the per-key cap is hit. Used by all mesh creation paths. - // Only active when flock.meshRecyclingEnabled is true. + // Used by all mesh creation paths; inert unless flock.meshRecyclingEnabled. _recycleOldestByKey(key) { if (!flock.meshRecyclingEnabled) return; if (!flock._modelInstances) flock._modelInstances = Object.create(null); @@ -2383,20 +2378,12 @@ export const flock = { }, removeEventListeners() {}, whenModelReady(id, callback) { - // Capture the current run's abort signal now. Guards must check this - // captured signal, never flock.abortController re-read at settle time: - // stopping a run aborts and replaces the controller, so a late-settling - // promise from the old run would see the new run's (unaborted) signal - // and run its stale callback into the new scene. + // Capture the signal now, never re-read flock.abortController at settle time: stopping a + // run replaces the controller, so a late promise would see the new run's unaborted signal. const signal = flock.abortController?.signal; - // --- Registry fast path --- - // Steady-state O(1): once a name has resolved to a live mesh it is - // cached in _liveNameCache (by announceMeshReady or a previous call - // here). A miss — or a stale entry whose mesh was disposed — falls - // through to the full lookup below, so worst case is the existing - // behaviour. Sentinel ids (__active_camera__/__main_light__) are never - // cached, so they take the normal path. + // Fast path: _liveNameCache. A miss, or a stale entry whose mesh was disposed, falls + // through to the full lookup. Sentinel ids are never cached, so they take the normal path. const cached = flock._ambiguousLiveNames?.has(id) ? null : flock._liveNameCache.get(id); if (cached) { if (cached.isDisposed()) { @@ -2428,20 +2415,13 @@ export const flock = { rejectP = reject; }); - // The callback often does async setup (e.g. awaiting materials, - // loading textures, or chaining other whenModelReady calls). - // Await it so the readiness promise only resolves once that - // user-provided async work is finished. const settle = async (val) => { if (settled) return; settled = true; try { - // Cache the resolved target (meshes only) so subsequent lookups of - // this id — including pre-sanitization aliases — skip the scans - // below and take the registry fast path. + // Cache the resolved target (meshes only) so later lookups take the fast path. flock._registerLiveName(id, val); - // Await the callback so the readiness promise doesn't resolve - // before user async work completes (premature resolution). + // Await the callback so the promise doesn't resolve before user async work completes. if (typeof callback === 'function') await callback(val); resolveP(val); } catch (error) { @@ -2465,21 +2445,16 @@ export const flock = { return t ?? null; }; - // --- Fast paths --- - // Runs at call time and again when a late-arriving scene appears (the - // rAF wait below), since the target may already be present by then. + // Runs at call time and again when a late-arriving scene appears (the rAF wait below). // Returns true when a path claimed the lookup and will settle. const tryFastPaths = () => { - // Check if there's a pending promise for this mesh first - // This ensures we wait for geometry to be attached before returning + // Prefer a pending promise, so we wait for geometry to be attached. if (flock.modelReadyPromises.has(id)) { const pendingPromise = flock.modelReadyPromises.get(id); pendingPromise .then((resolvedMesh) => { - // Re-locate after promise resolves to get mesh with geometry. - // Fall back to the resolved mesh value for alias lookups where - // id is the pre-sanitization name (e.g. "dimnnd monkey") but - // the actual mesh is named differently ("dimnndmonkey"). + // Re-locate for geometry; fall back to the resolved mesh for alias lookups where + // id is the pre-sanitization name ("dimnnd monkey" vs "dimnndmonkey"). const meshWithGeometry = locate() ?? resolvedMesh ?? null; if (!signal?.aborted) void settle(meshWithGeometry); }) @@ -2499,14 +2474,9 @@ export const flock = { } } - // --- Normalize-as-fallback for createCharacter/createObject names --- - // createCharacter and createObject strip special characters from names - // (e.g. "dimnnd monkey" → "dimnndmonkey"). createBox/createSphere etc. - // do NOT sanitize, so "centre platform" stays "centre platform". We must - // NOT apply normalization upfront or those box lookups break. - // Instead: only redirect to the normalized id when the exact id is absent - // but the normalized form IS present in modelReadyPromises or the scene. - // This also handles the stale-alias case (after the 5s TTL cleanup). + // createCharacter/createObject strip special characters but createBox etc. do not, so + // normalizing upfront would break box lookups ("centre platform" stays as typed). Only + // redirect to the normalized id when the exact one is absent. if (id && !id.startsWith('__')) { let norm = id.includes('__') ? id.split('__')[0] : id; norm = norm.replace(/[^a-zA-Z0-9._-]/g, ''); @@ -2585,10 +2555,9 @@ export const flock = { return; } - // main light — a plain flock property with no assignment event, so - // check per frame until it appears (mirrors the attachGui pattern). - // Mesh/GUI observers below could never match a light, so without - // this a waiter would pend until scene dispose. + // main light — a plain property with no assignment event, so poll per frame. The + // mesh/GUI observers below can never match a light, so without this a waiter pends + // until scene dispose. if (id === '__main_light__') { const lightNow = flock.mainLight; if (lightNow) { @@ -2619,14 +2588,8 @@ export const flock = { finish(mesh); return; } - // Handle the case where this observer was set up before - // createCharacter ran. createCharacter registers the - // pre-sanitization name (e.g. "dimnnd monkey") in - // modelReadyPromises pointing to the same promise as the - // sanitized name ("dimnndmonkey"). When any mesh is added - // to the scene (triggering this callback), check whether id - // is now in modelReadyPromises and if so switch to waiting - // for that promise instead. + // This observer may predate createCharacter registering id's pre-sanitization + // alias, so re-check modelReadyPromises and switch to that promise if it appeared. if (flock.modelReadyPromises?.has(id)) { done = true; while (disposers.length) { From 8c9883ad36e104b97907c1823bc53ac285c62895 Mon Sep 17 00:00:00 2001 From: Laura Sach <5183697+lawsie@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:28:49 +0100 Subject: [PATCH 4/4] Rabbit complaints --- flock.js | 2 +- main/blocklyinit.js | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/flock.js b/flock.js index c5404ce7..8a62e8e2 100644 --- a/flock.js +++ b/flock.js @@ -214,7 +214,7 @@ export const flock = { key = 'recursion_too_deep'; } // One fault can fire in a burst (stack-overflow cascade, per-frame loop error), so - // collapse identical reports within a short window. + // collapse within a short window when key, API and message match (`values` is not compared). const now = Date.now(); const sig = `${key}\0${api ?? ''}\0${safe?.message ?? ''}`; if (sig === flock._lastErrorSig && now - flock._lastErrorAt < 1000) { diff --git a/main/blocklyinit.js b/main/blocklyinit.js index 75f9b17c..1baa9504 100644 --- a/main/blocklyinit.js +++ b/main/blocklyinit.js @@ -375,8 +375,8 @@ function initializeIfClauseConnectionChecker(workspace) { return true; } - // Reject while dragging (visual feedback); otherwise allow the connection and let - // validateIfClausePositions disable the block in-place (healing, undo/redo, field change). + // Where a rule checks isDragging(), it rejects only mid-drag (visual feedback) and lets + // validateIfClausePositions disable the block in-place instead; other rules reject outright. if (connectingToNext) { // Moving block is connecting AFTER target @@ -1332,8 +1332,8 @@ export function initializeWorkspace() { } // Patch the Navigator so keyboard navigation skips the redundant block stop on value blocks -// whose only interactive content is a text-input field — shadow (" " in print_text) and -// standalone (`text`, `colour_from_string`) alike. +// with their own focusable field — shadow (" " in print_text) and standalone (`text`, +// `colour_from_string`) alike. Simple reporters are excluded; see the predicates below. function installShadowNavigationPatch(ws) { const nav = ws.getNavigator?.(); if (!nav) return; @@ -1363,8 +1363,8 @@ function installShadowNavigationPatch(ws) { !!node.outputConnection && !(typeof node.isSimpleReporter === 'function' && node.isSimpleReporter()); - // Variable/dropdown reporters are excluded (their primary field isn't a FieldTextInput), - // simple reporters for the same reason as isSkippableShadow. + // Excluded via isSimpleReporter (output + exactly one field) — same reason as + // isSkippableShadow. That is a field-count test, not a field-type one. const isSkippableStandalone = (node) => !!node?.outputConnection && !node.isShadow?.() && @@ -2451,8 +2451,9 @@ export function overrideSearchPlugin(workspace) { this.blockSearcher.indexBlocks = rebuildSearchIndex; blockSearcher.indexedBlocks_ = workspace.flockSearchIndexedBlocks || null; - // Override only the search primitive — Blockly's native matchBlocks consumes it, so - // flyout rendering, screen-reader announcements and no-match labels stay upstream. + // Override only the search primitive — native matchBlocks consumes it, so flyout rendering + // and screen-reader announcements stay upstream; matchBlocks is wrapped below to localise + // the placeholder and no-match labels. blockSearcher.blockTypesMatching = (rawQuery) => { const q = (rawQuery || '').toLowerCase().trim(); if (!q) return [];