Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 86 additions & 43 deletions packages/blockly/core/block_aria_composer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,18 @@ export enum ConnectionPreposition {
* The returned label will be specialized based on whether the block is part of a
* flyout.
*
* Custom input labels (from {@link Input.setAriaLabelProvider}) are not included
* here; they are used only in move-mode disambiguation and parent-input context
* via {@link Input.getAriaLabelText}.
*
* @internal
* @param block The block for which an ARIA representation should be created.
* @param verbosity How much detail to include in the description.
* @param useCustomInputLabels Whether to use custom labels for inputs, if they
* exist. We don't want to do this when just reading a block's label, but do
* want to in other scenarios such as move mode.
* @returns The ARIA representation for the specified block.
*/
export function computeAriaLabel(
block: BlockSvg,
verbosity = Verbosity.STANDARD,
useCustomInputLabels = true,
) {
if (block.isSimpleReporter()) {
// special case for full-block field blocks.
Expand All @@ -73,7 +73,7 @@ export function computeAriaLabel(
return [
verbosity >= Verbosity.STANDARD && getBeginStackLabel(block),
getParentInputLabel(block),
...getInputLabels(block, verbosity, useCustomInputLabels),
...getInputLabels(block, verbosity),
verbosity === Verbosity.LOQUACIOUS && getParentToolboxCategoryLabel(block),
verbosity >= Verbosity.STANDARD && getDisabledLabel(block),
verbosity >= Verbosity.STANDARD && getCollapsedLabel(block),
Expand Down Expand Up @@ -197,6 +197,10 @@ export function computeFieldRowLabel(
* statement input of the parent block (in this case, the label
* would be redundant with the parent block's label)
*
* For statement inputs without their own field labels, labels from other
* inputs in the same statement section are included (via
* {@link getInputLabelsSubset}), consistent with move-target disambiguation.
*
* For statement inputs, the resolved label (whether custom or fallback) is
* wrapped in the "Begin %1" prefix so the readout indicates that the child
* block starts the body of the statement input.
Expand Down Expand Up @@ -235,7 +239,14 @@ function getParentInputLabel(block: BlockSvg) {
return undefined;
}

inputLabel = computeFieldRowLabel(parentInput, true);
const sectionLabels = getInputLabelsSubset(
parentBlock as BlockSvg,
parentInput,
);
if (!sectionLabels.length) {
return undefined;
}
inputLabel = sectionLabels.join(', ');
}

if (parentInput.type === inputTypes.STATEMENT) {
Expand Down Expand Up @@ -272,21 +283,18 @@ function getBeginStackLabel(block: BlockSvg) {
* their contents are returned as a single item in the array per top-level
* input.
*
* Generally, if a custom label for an input is provided, that is preferred.
* However, we do not surface the custom labels when simply reading the text of
* the block. They are used as supplementary information for situations like
* move mode or when an input itself is focused.
* Uses derived labels only (field row text and connected block content via
* {@link Input.getLabel}). Custom input labels are not included; see
* {@link Input.getAriaLabelText} for move-mode and parent-input usage.
*
* @internal
* @param block The block to retrieve a list of field/input labels for.
* @param verbosity
* @param useCustomLabels whether to use the custom label for an input, if it's present.
* @param verbosity How much detail to include in each input label.
* @returns A list of field/input labels for the given block.
*/
export function getInputLabels(
block: BlockSvg,
verbosity = Verbosity.STANDARD,
useCustomLabels = true,
): string[] {
const visibleInputs = block.inputList.filter((input) => input.isVisible());
let inputsToLabel = visibleInputs;
Expand All @@ -306,15 +314,13 @@ export function getInputLabels(
}
}

return inputsToLabel.map((input) => {
const customLabel = useCustomLabels ? input.getAriaLabelText() : null;
return customLabel ?? input.getLabel(verbosity, useCustomLabels);
});
return inputsToLabel.map((input) => input.getLabel(verbosity));
}

/**
* Returns a subset of labels for inputs on the given block, ending at the
* specified input.
* Returns a subset of derived labels for inputs on the given block, ending at
* the specified input. Used to disambiguate move targets and connection
* highlights when no custom label is set.
*
* The subset is determined based on the input type:
* - For non-statement inputs, only the label for the given input is returned.
Expand All @@ -323,41 +329,74 @@ export function getInputLabels(
* begins immediately after the previous statement input, or at the start of
* the block if none exists.
*
* Label resolution (see also {@link computeMoveConnectionLabel}):
* 1. Custom labels ({@link Input.getAriaLabelText}) are handled by callers, not here.
* 2. Derived labels from {@link Input.getLabel} (field row + child blocks).
* 3. Numbered fallback ({@link Msg.INPUT_LABEL_INDEX}) when tier 2 is empty.
* For the statement target input, the fallback is omitted if any earlier
* input in the subset already produced a label.
*
* @internal
* @param block The block to retrieve a list of field/input labels for.
* @param input The input that defines the end of the subset.
* @returns A list of field/input labels for the given block.
*/
export function getInputLabelsSubset(
block: BlockSvg,
input: Input,
includeFallbackLabels = true,
): string[] {
export function getInputLabelsSubset(block: BlockSvg, input: Input): string[] {
const inputIndex = block.inputList.indexOf(input);
if (inputIndex === -1) {
throw new Error(
`Input with name "${input.name}" not found on block with id "${block.id}".`,
);
}
const isStatementTarget = input.type === inputTypes.STATEMENT;

const startIndex =
input.type === inputTypes.STATEMENT
? findStartOfStatementSection(block.inputList, inputIndex)
: inputIndex;
const startIndex = isStatementTarget
? findStartOfStatementSection(block.inputList, inputIndex)
: inputIndex;

return block.inputList
// For statement inputs, we include all visible inputs from the start
// of the current statement section up to and including the target input.
// For non-statement inputs, this will just be the target input itself.
const inputsInSubset = block.inputList
.slice(startIndex, inputIndex + 1)
.filter((input) => input.isVisible())
.map(
(input) =>
input.getLabel(Verbosity.TERSE, false) ||
(includeFallbackLabels
? Msg['INPUT_LABEL_INDEX'].replace(
'%1',
(input.getIndex() + 1).toString(),
)
: undefined),
)
.filter((subsetInput) => subsetInput.isVisible());

// The derived labels are based on the field row and any connected child
// blocks.
const derivedLabels = inputsInSubset.map((subsetInput) =>
subsetInput.getLabel(Verbosity.TERSE),
);

// For statement inputs, we only include the fallback label ("input %1")
// for the target input if no preceding input in the subset has a label.
// This prevents, e.g., "else" statement inputs from being read as "else, input 2".
const precedingLabelsProvideContext =
isStatementTarget && derivedLabels.slice(0, -1).some((label) => !!label);

return derivedLabels
.map((label, index) => {
if (label) {
return label;
}
const subsetInput = inputsInSubset[index];
// Dummy and end-row inputs are not connection inputs; getIndex() is -1
// and would produce a misleading "input 0" fallback label.
if (
subsetInput.type === inputTypes.DUMMY ||
subsetInput.type === inputTypes.END_ROW
) {
return undefined;
}
const isStatementTargetInput =
isStatementTarget && index === derivedLabels.length - 1;
if (isStatementTargetInput && precedingLabelsProvideContext) {
return undefined;
}
return Msg['INPUT_LABEL_INDEX'].replace(
'%1',
(subsetInput.getIndex() + 1).toString(),
);
})
.filter((label) => label !== undefined);
}

Expand Down Expand Up @@ -468,7 +507,13 @@ function getAnnouncementTemplate(preposition: ConnectionPreposition): string {
}

/**
* Returns a label for a connection includes either a block label, input label or both.
* Returns a label for a connection that includes either a block label, input
* label, or both.
*
* Input label resolution:
* 1. Custom label from {@link Input.getAriaLabelText} when set.
* 2. Otherwise derived labels from {@link getInputLabelsSubset} (field row,
* child blocks, and numbered fallbacks as needed).
*
* @param conn The connection to generate a label for.
* @param baseLabel An optional block label to include in the returned string.
Expand All @@ -483,8 +528,6 @@ function computeMoveConnectionLabel(

let inputLabel = input.getAriaLabelText();

// If the input doesn't have a custom ARIA label, compute one using the labels from
// nearby fields.
if (!inputLabel) {
const labels = getInputLabelsSubset(conn.getSourceBlock(), input);
if (!labels.length) return baseLabel;
Expand Down
4 changes: 2 additions & 2 deletions packages/blockly/core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2034,7 +2034,7 @@ export class BlockSvg
* @returns An accessibility description of this block.
*/
getAriaLabel(verbosity: aria.Verbosity) {
return computeAriaLabel(this, verbosity, false);
return computeAriaLabel(this, verbosity);
}

/**
Expand All @@ -2051,7 +2051,7 @@ export class BlockSvg
block = block.getNextBlock();
}
if (count <= 1) {
return computeAriaLabel(this, aria.Verbosity.TERSE, false);
return computeAriaLabel(this, aria.Verbosity.TERSE);
}

const labelTemplate = Msg['BLOCK_LABEL_STACK_BLOCKS'];
Expand Down
1 change: 0 additions & 1 deletion packages/blockly/core/dragging/block_drag_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,6 @@ export class BlockDragStrategy implements IDragStrategy {
* @param block The block to re-disable, if applicable.
*/
private redisableAllDraggedBlocks(block: BlockSvg) {
console.log('redisable');
const oldUndo = eventUtils.getRecordUndo();
eventUtils.setRecordUndo(false);
block.getDescendants(false).forEach((descendant) => {
Expand Down
27 changes: 26 additions & 1 deletion packages/blockly/core/field_image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,18 @@ export class FieldImage extends Field<string> {

if (config) {
this.configure_(config);
} else if (isFieldImageConfig(alt)) {
// Block Factory and some hand-written blocks pass a config object as the
// fourth argument instead of using the seventh `config` parameter.
// This is wrong, and typescript will complain about it, but handle it
// for backwards compatibility.
this.configure_(alt);
} else {
this.flipRtl = !!flipRtl;
this.altText = parsing.replaceMessageReferences(alt) || '';
this.altText =
typeof alt === 'string'
? parsing.replaceMessageReferences(alt) || ''
: '';
}
this.setValue(parsing.replaceMessageReferences(src));
}
Expand Down Expand Up @@ -372,6 +381,22 @@ export class FieldImage extends Field<string> {
}
}

/**
* Returns whether a value is a FieldImage config object passed in place of alt
* text (e.g. `{alt: '*', flipRtl: false}`). You shouldn't do this on purpose,
* but the block factory generates block definitions in this format.
*
* @param value The value to test.
*/
function isFieldImageConfig(value: unknown): value is FieldImageConfig {
return (
typeof value === 'object' &&
value !== null &&
Comment on lines +393 to +394
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.

Probably invert these two checks?

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.

that seems fine too, but just curious why?

Copy link
Copy Markdown
Contributor Author

@maribethb maribethb May 27, 2026

Choose a reason for hiding this comment

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

normally value is going to be a string, so having this check first will fail the fastest (desired behavior for the common case) though i'm not sure it really matters either way

!Array.isArray(value) &&
('alt' in value || 'flipRtl' in value)
);
}

fieldRegistry.register('field_image', FieldImage);

FieldImage.prototype.DEFAULT_VALUE = '';
Expand Down
15 changes: 6 additions & 9 deletions packages/blockly/core/inputs/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,13 +399,14 @@ export class Input {
}

/**
* Returns an accessibility label describing this input, including the labels
* of any fields on the input and the labels of any connected blocks, to help
* disambiguate this input from others on the same block.
* Returns a derived accessibility label for this input: field row text plus
* labels of any connected child blocks. Does not include custom labels from
* {@link getAriaLabelText}; those are used in move-mode and parent-input
* context only.
*
* @internal
*/
getLabel(verbosity = Verbosity.STANDARD, useCustomLabels = true): string {
getLabel(verbosity = Verbosity.STANDARD): string {
if (!this.isVisible()) return '';

const labels = computeFieldRowLabel(this, false, verbosity);
Expand All @@ -414,11 +415,7 @@ export class Input {
const childBlock = this.connection.targetBlock();
if (childBlock && !childBlock.isInsertionMarker()) {
labels.push(
getInputLabels(
childBlock as BlockSvg,
verbosity,
useCustomLabels,
).join(', '),
getInputLabels(childBlock as BlockSvg, verbosity).join(', '),
);
}
}
Expand Down
3 changes: 0 additions & 3 deletions packages/blockly/core/rendered_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,14 +355,11 @@ export class RenderedConnection

// Use the custom label for an input if it exists, otherwise use the
// "field row" approach to get the default label for the input.
// Don't include the "input 1" fallback for default labels, since
// the input is already being described as a statement or value input.
const parentInputLabel =
parentInput?.getAriaLabelText() ??
getInputLabelsSubset(
parentInput.getSourceBlock() as BlockSvg,
parentInput,
false,
).join(', ');
if (this.type === ConnectionType.NEXT_STATEMENT) {
aria.setState(
Expand Down
14 changes: 4 additions & 10 deletions packages/blockly/core/shortcut_items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -807,11 +807,7 @@ export function registerFocusToolbox() {
*/
export function registerReadInformation() {
const announceBlockInformation = (block: BlockSvg) => {
const description = computeAriaLabel(
block,
aria.Verbosity.LOQUACIOUS,
false,
);
const description = computeAriaLabel(block, aria.Verbosity.LOQUACIOUS);
aria.announceDynamicAriaState(description);
};

Expand Down Expand Up @@ -898,14 +894,12 @@ export function registerReadExtendedInformation() {
}

if (startBlock !== block) {
toAnnounce.push(
computeAriaLabel(startBlock, aria.Verbosity.TERSE, false),
);
toAnnounce.push(computeAriaLabel(startBlock, aria.Verbosity.TERSE));
}

let parent = startBlock.getParent();
while (parent) {
toAnnounce.push(computeAriaLabel(parent, aria.Verbosity.TERSE, false));
toAnnounce.push(computeAriaLabel(parent, aria.Verbosity.TERSE));
parent = parent.getParent();
}

Expand All @@ -917,7 +911,7 @@ export function registerReadExtendedInformation() {
toAnnounce.push(
Msg['CURRENT_BLOCK_ANNOUNCEMENT'].replace(
'%1',
computeAriaLabel(block, aria.Verbosity.TERSE, false),
computeAriaLabel(block, aria.Verbosity.TERSE),
),
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/blockly/demos/blockfactory/factory_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ FactoryUtils.getFieldsJs_ = function(block) {
}
break;
case 'field_image':
// Result: new Blockly.FieldImage('http://...', 80, 60, '*')
// Result: new Blockly.FieldImage('http://...', 80, 60, {alt: '*', flipRtl: false})
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.

Can this also be fixed to use the correct format moving forward? No worries if you're planning to do that in another PR or just file a bug for it

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.

this is the legacy block factory which we'll delete soon so not going to fix it here but can file a bug to fix it in samples

var src = JSON.stringify(block.getFieldValue('SRC'));
var width = Number(block.getFieldValue('WIDTH'));
var height = Number(block.getFieldValue('HEIGHT'));
Expand Down
Loading