refactor: Remove INavigable in favor of IFocusableNode. by gonfunko · Pull Request #9037 · RaspberryPiFoundation/blockly

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

gonfunko · 2025-05-13T20:47:41Z

The basics

I validated my changes

The details

Proposed Changes

This PR gets rid of the INavigable interface, since it was reduced to just getClass(), and there was significant objection to that. Instead, we now act directly on IFocusableNode instances. As a side effect, this also makes navigation policies Just Work for Field subclasses, and opens up the possibility of removing the cursor's curNode in favor of asking the focus manager for the currently focused thing, although I'll address that in a followup change. Registering navigation policies has been moved out of the cursor and into the Navigator.

BenHenning

Thanks @gonfunko! Had some thoughts.

Approving to unblock, but I'm happy to take another pass. Didn't look at the flyout navigation policy super closely since it appeared to have a bunch of unfinished code int.

BenHenning · 2025-05-13T21:00:23Z

+ * the given object.
+ *
+ * @param current An instance to check whether this policy applies to.
+ * @returns True if the given object is of a type handled by this policy.


A nit but I don't think this actually returns a boolean, I think it returns current coerced to type T or undefined if coercion fails per the predicate.
I'm not 100% certain on this and I find the TypeScript documentation unclear, but I recall debugging guards like this before and being surprised that their return values weren't actually booleans.

I agree the docs are unclear on this, but I think boolean is more useful to both folks reading and implementing than the perhaps technically correct value; ultimately the method needs to return a boolean (if you're implementing it) and you can rely on it being a truthy value if you call it.

I used the language 'whether' in this case which I suppose is slightly more ambiguous (perhaps beneficially):
/** * Determines whether the provided object fulfills the contract of * IFocusableNode. * * @param object The object to test. * @returns Whether the provided object can be used as an IFocusableNode. */exportfunctionisFocusableNode(object: any|null): object is IFocusableNode{return(object&&'getFocusableElement'inobject&&'getFocusableTree'inobject&&'onNodeFocus'inobject&&'onNodeBlur'inobject&&'canBeFocused'inobject);}
Not sure that's actually better. Note I don't have strong opinions here, so if you prefer the explicit 'True' that seems reasonable to me.

rachel-fenichel

Approved mod some deletion of logs.

This feels solid.

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

refactor: Remove INavigable in favor of IFocusableNode.
6e4f60f

gonfunko requested review from BenHenning and rachel-fenichel May 13, 2025 20:47

gonfunko assigned rachel-fenichel May 13, 2025

gonfunko requested a review from a team as a code owner May 13, 2025 20:47

github-actionsBot assigned BenHenning May 13, 2025

github-actionsBot added the PR: refactor Refactors code label May 13, 2025

chore: Fix JSDoc.
d4a36d8

This was referenced May 13, 2025
refactor: Update to use IFocusableNode in place of INavigable. RaspberryPiFoundation/blockly-keyboard-experimentation#520
Merged
fix: Ensure cursor syncs with more than just focused blocks #9032
Merged

BenHenning approved these changes May 13, 2025
View reviewed changes

BenHenning assigned gonfunko and unassigned BenHenning May 13, 2025

rachel-fenichel approved these changes May 13, 2025
View reviewed changes

Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated
Comment threadcore/keyboard_nav/flyout_navigation_policy.ts Outdated

chore: Address review feedback.
c99c117

gonfunko merged commit ae22165 into RaspberryPiFoundation:rc/v12.0.0May 13, 2025

gonfunko deleted the inavigable-refactor branch May 13, 2025 22:04

Uh oh!

Conversation

gonfunko commented May 13, 2025

The basics

The details

Proposed Changes

Uh oh!

BenHenning left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rachel-fenichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gonfunko commented May 13, 2025

The basics

The details

Proposed Changes

Uh oh!

BenHenning left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rachel-fenichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gonfunko commented May 13, 2025

The basics

The details

Proposed Changes

Uh oh!

BenHenning left a comment