fix: update keyboard nav to use proper getContents #2342
Conversation
|
The tests fail because navigation doesn't work with the current beta (7) because |
| if (firstFlyoutItem instanceof Blockly.FlyoutButton) { | ||
| const astNode = Blockly.ASTNode.createButtonNode(firstFlyoutItem); | ||
| if (!firstFlyoutItem) return; | ||
| if (firstFlyoutItem.type === 'button' && firstFlyoutItem.button) { |
There was a problem hiding this comment.
Is the type guaranteed to be lowercase? would it be possible to just check the property instead?
There was a problem hiding this comment.
core uses an enum so yes it will be lowercase. the enum isn't exported so i can't use it here. it's probably fine to just check the property... we don't actually use the type property anywhere afaik. i don't know why we have it. will update
There was a problem hiding this comment.
Sweet feel free to merge after update! or if you decide to not change it. Lemme know if you'd prefer re-review.
The basics
The details
Resolves
Needed for google/blockly#8064
Proposed Changes
Updates the keyboard-navigation plugin to use the API provided by
getContentsReason for Changes
Made a change to the type returned by
getContents. This hasn't been released yet other than in betas so this isn't a breaking change.Test Coverage
Tested by linking blockly & blockly-samples together to test both PRs and verified you can navigate buttons and blocks.
Documentation
Additional Information