Skip to content

fix: create more comprehensive field row labels #9458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: add-screen-reader-support-experimental
Choose a base branch
from
Open

fix: create more comprehensive field row labels #9458

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 11 additions & 3 deletions core/inputs/input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,17 @@ export class Input {
if (!fieldRowLabel) {
const inputs = this.getSourceBlock().inputList;
const index = inputs.indexOf(this);
if (index > 0) {
Copy link
Contributor

@BenHenning BenHenning Nov 4, 2025

Choose a reason for hiding this comment

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

This seems necessary to keep (at least, slice will probably misbehave without this check). That being said, I'm not sure it's even possible for this scenario to logically happen in practice, but it's definitely possible just from an API perspective.

Copy link
Contributor Author

@microbit-robert microbit-robert Nov 5, 2025

Choose a reason for hiding this comment

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

[1, 2, 3].slice(0, 0) returns [], so this should do no harm, but I'm happy to keep the check in.

return inputs[index - 1].getFieldRowLabel();
}
const precedingInputs = inputs.slice(0, index);
return precedingInputs
.flatMap((input) => {
const fields = input.fieldRow.map((field) => {
if (!field.isVisible()) return undefined;
return [field.getText() ?? field.getValue()];
});
return fields;
})
.filter(Boolean)
Copy link
Contributor

@BenHenning BenHenning Nov 4, 2025

Choose a reason for hiding this comment

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

I've actually never seen this syntax 'hack'. Is it effectively the same as .filter((value) => !!value)? Which, I suppose, is simply filtering out empty and null/undefined entries.

Copy link
Contributor Author

@microbit-robert microbit-robert Nov 5, 2025

Choose a reason for hiding this comment

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

Yes, this is exactly what it does in terms of API and in practice.

.join(' ');
}
return fieldRowLabel;
}
Expand Down
Loading