Support overrideKeystroke on static math #304
Conversation
jared-hughes
requested review from
eluberoff and
mikehaverstock
and removed request for
eluberoff
| if (controller.KIND_OF_MQ === 'StaticMath') { | ||
| controller.addTextareaEventListeners({ | ||
| keydown: (evt) => { | ||
| controller.options.overrideKeystroke?.(getMQKeyName(evt!), evt); |
There was a problem hiding this comment.
It feels a little weird to be using something named overrideKeystroke when this does not actually override the keystroke handling function. This is purely adding a listener, right? When we use overrideKeystroke elsewhere we do actually avoid calling the default handler.
There was a problem hiding this comment.
That is true, but this is on the StaticMath, which doesn't have an existing keystroke handling function. ('Copy' is not a keydown; it's a copy event). So it is overriding nothing with overrideKeystroke. I've added a comment to this effect.
src/services/textarea.ts
Outdated
| textarea.value = text; | ||
| if (text) textarea.select(); | ||
| }; | ||
| const textarea = this.getTextareaOrThrow(); |
There was a problem hiding this comment.
Are we guaranteed that the textarea exists at this point and that it won't change? Do we not delay instantiation of that textarea ever? I'm wondering why we might sometimes throw when fetching the textarea and if it's a good idea to throw in the middle of setting up a mathquill.
There was a problem hiding this comment.
Yes, we're guaranteed that the textarea exists and it doesn't change. Since #303, we are no longer adding and removing a textarea in static math. I've added some comments noting the order of initialization in commands/math.ts.
There was a problem hiding this comment.
Can we remove the "OrThrow" from the name then?
There was a problem hiding this comment.
Sure, I've done that. Kept the throw in place though (converted it to a pray). Better an early error there instead of a ! assertion.
jared-hughes
force-pushed
the
jared/static-saneKeyboardEvents
branch
from
1faf843 to
6a07b06
Compare
Convert throw to pray.
No description provided.