google/blockly

Keyboard navigation should only use methods available on IFlyout

Closed this issue · 3 comments

Check for duplicates

  • I have searched for similar issues before opening a new one.

Problem

In MakeCode, we implement a custom flyout that does not extend the base Flyout abstract class. In the keyboard navigation AST code, the flyout returned by workspace.getFlyout() is cast as a Flyout rather than an IFlyout, and uses methods that are implemented on that class but not included in the IFlyout interface.

For example, in this function, getContents() is part of Flyout and not IFlyout

Request

Keyboard navigation should use the IFlyout interface rather than the Flyout class to ensure compatibility with plugin contributed flyouts.

Alternatives considered

For now, I've just implemented the getContents() method on our Flyout.

Additional context

No response

@riknoll fyi I think we have to keep this as a breaking change to require getContents in order to support accurate keyboard navigation. So I've formalized that in #8064 by adding it to the interface and removing the cast to Flyout.

Wanted to give you a heads up as I've changed the typing. I found your implementation and I think you'll just need to update your own typings of the function as well as remove the casts. Let me know if you anticipate problems with this. Thanks!

Without having looked at it closely, this seems perhaps related to google/blockly-samples#399. Just commenting to cross-link the two for future reference.

@maribethb should be no problem for us, thanks for the fix!