From c90f05cb942de9475ca6415f8b183636faa00866 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 14 May 2021 21:21:58 -0400 Subject: [PATCH] Fix KeyboardNavigatable In acd3873bbd46e92510ebf92e7182324e1c6d540e, 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 --- framework/core/js/src/forum/utils/KeyboardNavigatable.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/framework/core/js/src/forum/utils/KeyboardNavigatable.ts b/framework/core/js/src/forum/utils/KeyboardNavigatable.ts index 1e9dc5568..739a590fc 100644 --- a/framework/core/js/src/forum/utils/KeyboardNavigatable.ts +++ b/framework/core/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; } /**