From 05dda5b083e0a52894ef30dcb2cb13331cd74025 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 14 May 2021 21:21:58 -0400 Subject: [PATCH] Fix KeyboardNavigatable In b2d053f6865e685ebf005e457d970385377bbb28, I tried to be clever and create a new KeyboardNavigatable object as a return value for `when`. My approach to cloning was incorrect, and caused the util to break entirely. My original intent for having this "clone"-based behavior is that a single KeyboardNavigatable instance could be created with multiple listeners, and then "cloned" like this with different "activators" registered via "then" calls. In hindsight, this change introduces more issues than it solves: outside of just not working, the cloned "KeyboardNavigatable" instances have shared internal state (the set of callbacks), and each has write access to this internal state. This is a recipe for unpredictable behavior and confusing bugs, so best to keep things simple for now, and maybe introduce more functional behavior in later releases. Fixes https://github.com/flarum/QualityAssurance/issues/25 --- js/src/forum/utils/KeyboardNavigatable.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/js/src/forum/utils/KeyboardNavigatable.ts b/js/src/forum/utils/KeyboardNavigatable.ts index 1e9dc5568..739a590fc 100644 --- a/js/src/forum/utils/KeyboardNavigatable.ts +++ b/js/src/forum/utils/KeyboardNavigatable.ts @@ -104,7 +104,9 @@ export default class KeyboardNavigatable { * Provide a callback that determines whether keyboard input should be handled. */ when(callback: ShouldHandle): KeyboardNavigatable { - return { ...this, whenCallback: callback }; + this.whenCallback = callback; + + return this; } /**