fix: improve block labels and aria roles#9834
fix: improve block labels and aria roles#9834maribethb wants to merge 1 commit intoRaspberryPiFoundation:v13from
Conversation
| @@ -2002,6 +2003,7 @@ export class BlockSvg | |||
| * Updates the ARIA label, role and roledescription for this block. | |||
| */ | |||
| private recomputeAriaAttributes() { | |||
There was a problem hiding this comment.
Elsewhere, these methods are called recomputeAriaContext. Worth renaming?
| try { | ||
| focusableElement = this.getFocusableElement(); | ||
| } catch { | ||
| // just return because the field hasn't been init yet |
There was a problem hiding this comment.
Nit: Capitalize and add period if this will be around long-term.
| // top-level full-block fields in the flyout need to have their | ||
| // roledescription set. this can't happen in the flyout code because | ||
| // the field hasn't been initialized yet then. | ||
| // these blocks should also have the rest of the state in this method set. |
| aria.setRole(focusableElement, aria.Role.PRESENTATION); | ||
| aria.clearState(focusableElement, aria.State.LABEL); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Are we no longer using the button role for clickable images? It possible for an image field to become clickable after it's made? If so, would the outdated 'presentation' role need to be cleared?
If we are changing things here, we should probably also update or simplify the comment above.
| let label = this.computeAriaLabel(true); | ||
|
|
||
| if (this.isCurrentlyEditable?.()) { | ||
| if (this.isCurrentlyEditable() && !this.getSourceBlock()?.isInFlyout) { |
There was a problem hiding this comment.
If we're in a flyout, would we have already returned false earlier above? Not sure we need to check .isInFlyout again.
|
|
||
| test('Blocks without inputs are properly labeled', function () { | ||
| const block = this.makeBlock('math_random_float'); | ||
| const block = this.makeBlock('logic_boolean'); |
There was a problem hiding this comment.
Any particular reason for this change?
| suite('of bubbles', function () { | ||
| setup(async function () { | ||
| const commentBlock = this.workspace.newBlock('logic_boolean'); | ||
| const commentBlock = this.workspace.newBlock('logic_compare'); |
There was a problem hiding this comment.
Also curious about this change.
2 participants
The basics
The details
Resolves
Fixes #9802
Fixes #9804
Partially addresses #9793 (actually does fix it technically but making a follow up change related)
Fixes #9790
Fixes RaspberryPiFoundation/blockly-keyboard-experimentation#789
Proposed Changes
getFullBlockFieldmethod to facilitate checking this. this is different than what's in the design doc for reasons i put in a comment in the design doc.isSimpleReportermethod to better explain what that means.fieldclass and had subclasses override it where needed to set customized labels and other states.Reason for Changes
Makes aria markup better
Test Coverage
Fixed the tests, removed some that assert behavior we don't actually want.
I skipped some tests related to custom input labels because I'm about to implement the custom input labels in a separate PR and I'll unskip them then.
Documentation
maybe for setting aria for custom fields, but it depends how we write the documentation.
Additional Information