From b70059064a5ac70ae5c46798a2fe2a820666f023 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 25 Jun 2024 16:18:20 -0700 Subject: [PATCH 1/9] Allow core logging functions to accept multiple arguments. --- kolibri/core/assets/src/logging.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kolibri/core/assets/src/logging.js b/kolibri/core/assets/src/logging.js index 902174e19d1..38e15cce507 100644 --- a/kolibri/core/assets/src/logging.js +++ b/kolibri/core/assets/src/logging.js @@ -14,8 +14,8 @@ class Logger { const name = methodName.toLowerCase(); const logFunction = this.logger[name]; if (logFunction) { - this[name] = param => { - return this.logger[name](param); + this[name] = (...params) => { + return this.logger[name](...params); }; } }); @@ -25,8 +25,8 @@ class Logger { var originalFactory = this.logger.methodFactory; this.logger.methodFactory = function (methodName, logLevel, loggerName) { var rawMethod = originalFactory(methodName, logLevel, loggerName); - return function (message) { - rawMethod(`[${methodName.toUpperCase()}: ${loggerName}] ` + message); + return function (message, ...args) { + rawMethod(`[${methodName.toUpperCase()}: ${loggerName}] ` + message, ...args); }; }; this.logger.setLevel(this.logger.getLevel()); @@ -55,7 +55,7 @@ class Logging { this.defaultLogger = new Logger('root'); Object.keys(loglevel.levels).forEach(methodName => { const name = methodName.toLowerCase(); - this[name] = msg => this.defaultLogger[name](msg); + this[name] = (...msgs) => this.defaultLogger[name](...msgs); }); } From 3f988f850d7dfadce0058872ee98bf2fe840aa64 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 25 Jun 2024 16:18:42 -0700 Subject: [PATCH 2/9] Sanitize question state before attempting to restore it. --- .../assets/src/views/PerseusRendererIndex.vue | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index 507138d20eb..cb928db287d 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -537,8 +537,20 @@ answerState = JSON.parse( replaceImageUrls(JSON.stringify(answerState), this.perseusFileUrl), ); + const widgetIds = this.itemRenderer.getWidgetIds(); + // Because of a switch between the input-number and numeric-input widgets + // it seems it is possible for us to have a serialized state with keys + // that do not correspond to any widgets. We need to sanitize the state + // before restoring it. + const sanitizedQuestions = {}; + for (const key of widgetIds) { + if (answerState.question[key]) { + sanitizedQuestions[key] = answerState.question[key]; + } + } + answerState.question = sanitizedQuestions; this.itemRenderer.restoreSerializedState(answerState); - this.itemRenderer.getWidgetIds().forEach(id => { + widgetIds.forEach(id => { if (sorterWidgetRegex.test(id)) { if (answerState.question[id]) { const sortableComponent = From af14d94a9e62b6ff3a0c3915ac13fbce6680ea91 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 25 Jun 2024 16:19:08 -0700 Subject: [PATCH 3/9] Ensure all listeners are setup before calling render to ensure answer gets set. --- .../perseus_viewer/assets/src/views/PerseusRendererIndex.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index cb928db287d..1bed1876e66 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -453,7 +453,6 @@ render(renderStateRootElement, this.$refs.perseus); }, renderNewItem() { - this.renderItem(); // Clear any pending state reset calls this.$off('itemRendererUpdated'); this.$once('itemRendererUpdated', () => { @@ -466,6 +465,7 @@ // so we need to ensure that the itemRenderer is available and up to date first. this.setAnswer(); }); + this.renderItem(); }, _resetState(val) { if (!val) { From 06dcb164383c1599f8c7b7fa4e0597951195daab Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 27 Jun 2024 12:55:41 -0700 Subject: [PATCH 4/9] Tweak variable naming to reduce confusion. --- .../assets/src/views/PerseusRendererIndex.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index 1bed1876e66..fd39ab19559 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -542,13 +542,13 @@ // it seems it is possible for us to have a serialized state with keys // that do not correspond to any widgets. We need to sanitize the state // before restoring it. - const sanitizedQuestions = {}; + const sanitizedQuestion = {}; for (const key of widgetIds) { if (answerState.question[key]) { - sanitizedQuestions[key] = answerState.question[key]; + sanitizedQuestion[key] = answerState.question[key]; } } - answerState.question = sanitizedQuestions; + answerState.question = sanitizedQuestion; this.itemRenderer.restoreSerializedState(answerState); widgetIds.forEach(id => { if (sorterWidgetRegex.test(id)) { From 62c7440b958cfdb52dbdb0d3bc8cf37af6cd598d Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 1 Jul 2024 09:09:00 -0700 Subject: [PATCH 5/9] Dismiss the keypad between item renders to prevent unmounted component errors. --- .../assets/src/views/PerseusRendererIndex.vue | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index fd39ab19559..2fe8826dfdb 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -293,6 +293,7 @@ }, created() { this.itemRenderer = null; + this.keypadElement = null; // This is a local object for tracking image URLs // we use this to clean up image URLs just for this component this.imageUrls = {}; @@ -412,22 +413,24 @@ const keypadContextConsumerElement = e( KeypadContext.Consumer, { key: 'keypadContextConsumer' }, - ({ keypadElement }) => - e(perseus.ServerItemRenderer, { + ({ keypadElement }) => { + this.keypadElement = keypadElement; + return e(perseus.ServerItemRenderer, { ...itemRenderData, keypadElement: keypadElement, - }), + }); + }, ); const keypadWithContextElement = e( KeypadContext.Consumer, { key: 'keypadWithContext ' }, - ({ setKeypadElement }) => + ({ setKeypadElement, renderer }) => e('div', { className: 'keypad-container', children: e(MobileKeypad, { style: keypadStyle.keypadContainer, onElementMounted: setKeypadElement, - onDismiss: () => {}, + onDismiss: () => renderer && renderer.blur(), onAnalyticsEvent: async () => {}, }), }), @@ -455,6 +458,10 @@ renderNewItem() { // Clear any pending state reset calls this.$off('itemRendererUpdated'); + // Dismiss the keypad + if (this.keypadElement) { + this.keypadElement.dismiss(); + } this.$once('itemRendererUpdated', () => { // Blur any previously focused element once we have rendered a new item this.itemRenderer.blur(); From cfb38ebb32e43c64da718f2c018100ac5c7f18dc Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 1 Jul 2024 09:11:29 -0700 Subject: [PATCH 6/9] Don't show keypad if the renderer is not interactive. --- .../perseus_viewer/assets/src/views/PerseusRendererIndex.vue | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index 2fe8826dfdb..525ffaa0ccd 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -391,7 +391,8 @@ onFocusChange: this.dismissMessage, onInputError: logging.error, isMobile: this.isMobile, - customKeypad: true, + // Only show a keypad if we are in interactive mode. + customKeypad: this.interactive, readOnly: !this.interactive, hintProgressColor: this.$themeTokens.primary, }, From 7483d1e6c7188db737b900e0d4479b495aa4c0d4 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 1 Jul 2024 09:34:43 -0700 Subject: [PATCH 7/9] Simplify keypad positioning to be relative to parent. --- .../assets/src/views/PerseusRendererIndex.vue | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index 525ffaa0ccd..18dc098c07b 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -56,7 +56,7 @@ const keypadStyle = StyleSheet.create({ keypadContainer: { - position: 'relative', + position: 'absolute', }, }); @@ -426,14 +426,11 @@ KeypadContext.Consumer, { key: 'keypadWithContext ' }, ({ setKeypadElement, renderer }) => - e('div', { - className: 'keypad-container', - children: e(MobileKeypad, { - style: keypadStyle.keypadContainer, - onElementMounted: setKeypadElement, - onDismiss: () => renderer && renderer.blur(), - onAnalyticsEvent: async () => {}, - }), + e(MobileKeypad, { + style: keypadStyle.keypadContainer, + onElementMounted: setKeypadElement, + onDismiss: () => renderer && renderer.blur(), + onAnalyticsEvent: async () => {}, }), ); const statefulKeypadContextProviderElement = e(StatefulKeypadContextProvider, { @@ -973,10 +970,4 @@ } } - .keypad-container { - flex-shrink: 0; /* Prevent the keypad from shrinking */ - justify-content: center; /* Center the keypad horizontally */ - direction: ltr; - } - From 7a19ace15363c6bb9b8a8bcae66cf55d60e0bfe6 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 2 Jul 2024 09:49:11 -0700 Subject: [PATCH 8/9] Portal the keypad and have it positioned fixed, so that it can always overlay everything. --- .../assets/src/views/PerseusRendererIndex.vue | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index 18dc098c07b..694bd944672 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -37,7 +37,7 @@ import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow'; import { defer } from 'underscore'; import { createElement as e } from 'react'; - import { render, unmountComponentAtNode } from 'react-dom'; + import { createPortal, render, unmountComponentAtNode } from 'react-dom'; import * as perseus from '@khanacademy/perseus'; import { MathInputI18nContextProvider, @@ -56,7 +56,7 @@ const keypadStyle = StyleSheet.create({ keypadContainer: { - position: 'absolute', + zIndex: 20, }, }); @@ -426,12 +426,15 @@ KeypadContext.Consumer, { key: 'keypadWithContext ' }, ({ setKeypadElement, renderer }) => - e(MobileKeypad, { - style: keypadStyle.keypadContainer, - onElementMounted: setKeypadElement, - onDismiss: () => renderer && renderer.blur(), - onAnalyticsEvent: async () => {}, - }), + createPortal( + e(MobileKeypad, { + style: keypadStyle.keypadContainer, + onElementMounted: setKeypadElement, + onDismiss: () => renderer && renderer.blur(), + onAnalyticsEvent: async () => {}, + }), + document.body, + ), ); const statefulKeypadContextProviderElement = e(StatefulKeypadContextProvider, { children: [keypadContextConsumerElement, keypadWithContextElement], From faaca33f9c44909b0c82f415b3cd765f80ed9d96 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Tue, 2 Jul 2024 10:01:04 -0700 Subject: [PATCH 9/9] Don't pass the keypad element when not interactive to hide keypad in static presentation. --- .../assets/src/views/PerseusRendererIndex.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue index 694bd944672..43822b9290c 100644 --- a/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/assets/src/views/PerseusRendererIndex.vue @@ -391,8 +391,8 @@ onFocusChange: this.dismissMessage, onInputError: logging.error, isMobile: this.isMobile, - // Only show a keypad if we are in interactive mode. - customKeypad: this.interactive, + // Always use our custom keypad implementation + customKeypad: true, readOnly: !this.interactive, hintProgressColor: this.$themeTokens.primary, }, @@ -418,7 +418,7 @@ this.keypadElement = keypadElement; return e(perseus.ServerItemRenderer, { ...itemRenderData, - keypadElement: keypadElement, + keypadElement: this.interactive ? keypadElement : null, }); }, );