From 8c8df7740be54d2e64258e83460cedb97043951f Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 29 Sep 2025 16:46:42 -0700 Subject: [PATCH 1/5] Refactor content viewer to no longer require implemented viewer components to use the viewer props, and instead direct all data via the composable api. --- .../single_page_apps.rst | 6 +- kolibri/core/content/hooks.py | 32 +- kolibri/core/content/zip_wsgi.py | 3 +- .../frontend/views/BloomPubRendererIndex.vue | 7 +- .../views/common/QuestionsAccordion.vue | 4 +- .../common/reports/QuestionLearnersPage.vue | 4 +- .../LessonContentPreview/ContentArea.vue | 3 +- .../CreateExamPage/QuestionListPreview.vue | 4 +- .../frontend/views/EpubRendererIndex.vue | 7 +- kolibri/plugins/epub_viewer/kolibri_plugin.py | 1 + .../frontend/views/Html5AppRendererIndex.vue | 27 +- .../views/AssessmentWrapper/index.vue | 45 ++- .../views/ChannelRenderer/ContentItem.vue | 15 +- .../learn/frontend/views/ContentPage.vue | 11 +- .../CourseUnitView/CourseContentViewer.vue | 6 +- .../learn/frontend/views/ExamPage/index.vue | 4 +- .../frontend/views/QuizRenderer/index.vue | 4 +- .../__tests__/assessment-wrapper.spec.js | 10 +- .../__tests__/AssessmentWrapper.spec.js | 85 ++-- .../views/courses/AssessmentWrapper/index.vue | 30 +- .../frontend/utils/fileExtractors.js | 67 ++++ .../frontend/views/MediaPlayerIndex.vue | 28 +- .../plugins/media_player/kolibri_plugin.py | 2 + .../frontend/views/PdfRendererIndex.vue | 23 +- .../views/__tests__/PdfRendererIndex.spec.js | 24 +- kolibri/plugins/pdf_viewer/kolibri_plugin.py | 1 + .../frontend/views/PerseusRendererIndex.vue | 20 +- .../plugins/perseus_viewer/kolibri_plugin.py | 1 + .../frontend/components/QTIViewer.vue | 31 +- kolibri/plugins/qti_viewer/kolibri_plugin.py | 1 + .../frontend/views/SafeHtml5RendererIndex.vue | 15 +- .../safe_html5_viewer/kolibri_plugin.py | 1 + .../views/SlideshowRendererComponent.vue | 10 +- .../SafeHTML/__tests__/SafeHTML.spec.js | 271 +++++++++++++ .../components/SafeHTML/index.js | 91 +++-- .../components/quizzes/QuizReport/index.vue | 4 +- packages/kolibri-zip/src/fileUtils.js | 2 +- .../__tests__/DownloadButton.spec.js | 9 +- .../ContentViewer/ContentViewerError.vue | 2 +- .../__tests__/contentViewer.spec.js | 34 +- .../internal/ContentViewer/index.js | 314 +++++++++++++-- .../internal/ContentViewer/utils.js | 15 +- .../composables/__mocks__/useContentViewer.js | 85 ++++ .../kolibri/composables/useContentViewer.js | 376 ++++++++++-------- packages/kolibri/constants.js | 2 - packages/kolibri/internal/apiSpec.js | 1 + packages/kolibri/internal/pluginMediator.js | 124 ++++-- packages/kolibri/package.json | 1 + .../styles/internal/initializeTheme.js | 8 +- packages/kolibri/urls.js | 14 +- .../kolibri/utils/__tests__/mimetypes.spec.js | 96 +++++ packages/kolibri/utils/mimetypes.js | 91 +++++ 52 files changed, 1583 insertions(+), 489 deletions(-) create mode 100644 kolibri/plugins/media_player/frontend/utils/fileExtractors.js create mode 100644 packages/kolibri-common/components/SafeHTML/__tests__/SafeHTML.spec.js create mode 100644 packages/kolibri/composables/__mocks__/useContentViewer.js create mode 100644 packages/kolibri/utils/__tests__/mimetypes.spec.js create mode 100644 packages/kolibri/utils/mimetypes.js diff --git a/docs/frontend_architecture/single_page_apps.rst b/docs/frontend_architecture/single_page_apps.rst index 8bdcdfe172f..b38ece7b541 100644 --- a/docs/frontend_architecture/single_page_apps.rst +++ b/docs/frontend_architecture/single_page_apps.rst @@ -92,13 +92,13 @@ A special kind of Kolibri Module is dedicated to rendering particular content ty :members: :noindex: -The ``ContentViewer`` class has one required property ``viewerComponent`` which should return a Vue component that wraps the content rendering code. This component will be passed ``files``, ``file``, ``itemData``, ``preset``, ``itemId``, ``answerState``, ``allowHints``, ``extraFields``, ``interactive``, ``lang``, ``showCorrectAnswer``, ``defaultItemPreset``, ``availableFiles``, ``defaultFile``, ``supplementaryFiles``, ``thumbnailFiles``, ``contentDirection``, and ``contentIsRtl`` props, defining the files associated with the piece of content, and other required data for rendering. +The ``ContentViewer`` class has one required property ``viewerComponent`` which should return a Vue component that wraps the content rendering code. The ``ContentViewer`` wrapper accepts a ``contentNode`` prop containing the content metadata (including ``files``, ``lang``, ``options``, and ``duration``), and provides all necessary data to descendant viewer components via the ``useContentViewer`` composable. -The component should use the `useContentViewer` composable and the `contentViewerProps`, importable as follows: +The component should use the `useContentViewer` composable, importable as follows: .. code-block:: javascript - import useContentViewer, { contentViewerProps } from 'kolibri/composables/useContentViewer'; + import useContentViewer from 'kolibri/composables/useContentViewer'; In order to log data about users viewing content, the component should emit ``startTracking``, ``updateProgress``, and ``stopTracking`` events, using the Vue ``$emit`` method. ``startTracking`` and ``stopTracking`` are emitted without any arguments, whereas ``updateProgress`` should be emitted with a single value between 0 and 1 representing the current proportion of progress on the content. diff --git a/kolibri/core/content/hooks.py b/kolibri/core/content/hooks.py index 2d72566933a..d9fd2b6e1a5 100644 --- a/kolibri/core/content/hooks.py +++ b/kolibri/core/content/hooks.py @@ -9,6 +9,8 @@ from django.core.serializers.json import DjangoJSONEncoder from django.utils.safestring import mark_safe +from le_utils.constants import file_formats +from le_utils.constants import format_presets from kolibri.core.webpack.hooks import WebpackBundleHook from kolibri.core.webpack.hooks import WebpackInclusionMixin @@ -29,6 +31,30 @@ class ContentRendererHook(WebpackBundleHook, WebpackInclusionMixin): def presets(self): pass + #: Optional tuple of CSS selectors that this content renderer can handle + css_selectors = () + + #: Whether to allow object tag handling (defaults to False for sandboxing compatibility) + allow_object_tag = False + + @classmethod + def all_css_selectors(cls): + """Get all CSS selectors (auto-generated from presets + custom), cached.""" + if not hasattr(cls, "_cached_css_selectors"): + selectors = list(cls.css_selectors) + + if cls.allow_object_tag: + for preset in cls.presets: + preset_obj = next( + x for x in format_presets.PRESETLIST if x.id == preset + ) + for fmt in preset_obj.allowed_formats: + fmt_obj = file_formats.getformat(fmt) + selectors.append(f'object[type="{fmt_obj.mimetype}"]') + + cls._cached_css_selectors = tuple(sorted(set(selectors))) + return cls._cached_css_selectors + @classmethod def html(cls): tags = [] @@ -53,7 +79,11 @@ def template_html(self): ''.format( bundle=self.unique_id, data=json.dumps( - {"urls": urls, "presets": self.presets}, + { + "urls": urls, + "presets": self.presets, + "css_selectors": self.all_css_selectors(), + }, separators=(",", ":"), ensure_ascii=False, cls=DjangoJSONEncoder, diff --git a/kolibri/core/content/zip_wsgi.py b/kolibri/core/content/zip_wsgi.py index 144d3339184..67e2f2ce4c3 100644 --- a/kolibri/core/content/zip_wsgi.py +++ b/kolibri/core/content/zip_wsgi.py @@ -24,6 +24,7 @@ from le_utils.constants.file_formats import BLOOMPUB from le_utils.constants.file_formats import H5P from le_utils.constants.file_formats import HTML5 +from le_utils.constants.file_formats import HTML5_ARTICLE from le_utils.constants.file_formats import PERSEUS from whitenoise.responders import StaticFile @@ -270,7 +271,7 @@ def get_embedded_file( return response -archive_file_types = (HTML5, H5P, BLOOMPUB, BLOOMD, PERSEUS) +archive_file_types = (HTML5, H5P, BLOOMPUB, BLOOMD, PERSEUS, HTML5_ARTICLE) archive_file_extension_match = "|".join(archive_file_types) # Allows a base url to be passed in the main diff --git a/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue b/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue index 4e5c7cf9927..2cae0364fd8 100644 --- a/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue +++ b/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue @@ -59,7 +59,7 @@ import { now } from 'kolibri/utils/serverClock'; import CoreFullscreen from 'kolibri-common/components/CoreFullscreen'; import Sandbox from 'kolibri-sandbox'; - import useContentViewer, { contentViewerProps } from 'kolibri/composables/useContentViewer'; + import useContentViewer from 'kolibri/composables/useContentViewer'; const defaultContentHeight = '500px'; const frameTopbarHeight = '37px'; @@ -69,18 +69,17 @@ CoreFullscreen, }, setup(props, context) { - const { defaultFile, forceDurationBasedProgress, reportError } = useContentViewer( - props, + const { defaultFile, options, forceDurationBasedProgress, reportError } = useContentViewer( context, { defaultDuration: 300 }, ); return { defaultFile, + options, forceDurationBasedProgress, reportError, }; }, - props: contentViewerProps, data() { return { iframeHeight: (this.options && this.options.height) || defaultContentHeight, diff --git a/kolibri/plugins/coach/frontend/views/common/QuestionsAccordion.vue b/kolibri/plugins/coach/frontend/views/common/QuestionsAccordion.vue index 5dcaff78447..b3d958116fc 100644 --- a/kolibri/plugins/coach/frontend/views/common/QuestionsAccordion.vue +++ b/kolibri/plugins/coach/frontend/views/common/QuestionsAccordion.vue @@ -99,10 +99,8 @@ diff --git a/kolibri/plugins/coach/frontend/views/lessons/LessonSelectionContentPreviewPage/LessonContentPreview/ContentArea.vue b/kolibri/plugins/coach/frontend/views/lessons/LessonSelectionContentPreviewPage/LessonContentPreview/ContentArea.vue index 0e6be991caf..620de90040b 100644 --- a/kolibri/plugins/coach/frontend/views/lessons/LessonSelectionContentPreviewPage/LessonContentPreview/ContentArea.vue +++ b/kolibri/plugins/coach/frontend/views/lessons/LessonSelectionContentPreviewPage/LessonContentPreview/ContentArea.vue @@ -10,11 +10,10 @@ props.contentNode.assessmentmetadata.assessment_item_ids, + ); + const randomize = computed(() => props.contentNode.assessmentmetadata.randomize); + const masteryModel = computed(() => props.contentNode.assessmentmetadata.mastery_model); return { windowIsSmall, currentUserId, + assessmentIds, + randomize, + masteryModel, }; }, props: { - ...contentViewerProps, - assessmentIds: { - type: Array, - required: true, - }, - randomize: { - type: Boolean, + contentNode: { + type: Object, required: true, }, - masteryModel: { + extraFields: { type: Object, - required: true, + default: () => ({}), + }, + progress: { + type: Number, + default: 0, + }, + userId: { + type: String, + default: '', + }, + userFullName: { + type: String, + default: '', + }, + timeSpent: { + type: Number, + default: 0, }, pastattempts: { type: Array, diff --git a/kolibri/plugins/learn/frontend/views/ChannelRenderer/ContentItem.vue b/kolibri/plugins/learn/frontend/views/ChannelRenderer/ContentItem.vue index 1cd83c7a79d..b8814f01959 100644 --- a/kolibri/plugins/learn/frontend/views/ChannelRenderer/ContentItem.vue +++ b/kolibri/plugins/learn/frontend/views/ChannelRenderer/ContentItem.vue @@ -95,11 +95,7 @@ return {}; } return { - kind: this.contentNode.kind, - lang: this.contentNode.lang, - files: this.contentNode.files, - options: this.contentNode.options, - available: this.contentNode.available, + contentNode: this.contentNode, extraFields: this.extra_fields, progress: this.progress, userId: this.currentUserId, @@ -111,15 +107,8 @@ if (!this.contentIsExercise) { return {}; } - const assessment = this.contentNode.assessmentmetadata || {}; return { - kind: this.contentNode.kind, - files: this.contentNode.files, - lang: this.contentNode.lang, - randomize: this.contentNode.randomize, - masteryModel: assessment.mastery_model, - assessmentIds: assessment.assessment_item_ids, - available: this.contentNode.available, + contentNode: this.contentNode, extraFields: this.extra_fields, progress: this.progress, userId: this.currentUserId, diff --git a/kolibri/plugins/learn/frontend/views/ContentPage.vue b/kolibri/plugins/learn/frontend/views/ContentPage.vue index b0cb742638c..02b57b49840 100644 --- a/kolibri/plugins/learn/frontend/views/ContentPage.vue +++ b/kolibri/plugins/learn/frontend/views/ContentPage.vue @@ -5,10 +5,7 @@ { const propsData = { id: 'test', kind: 'test', - assessmentIds: [], - masteryModel: masteryModel || {}, - randomize: false, + contentNode: { + assessmentmetadata: { + assessment_item_ids: [], + mastery_model: masteryModel || {}, + randomize: false, + }, + }, totalattempts, pastattempts, }; diff --git a/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/__tests__/AssessmentWrapper.spec.js b/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/__tests__/AssessmentWrapper.spec.js index 47d437e1ffb..66da10c8a8f 100644 --- a/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/__tests__/AssessmentWrapper.spec.js +++ b/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/__tests__/AssessmentWrapper.spec.js @@ -1,39 +1,55 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/vue'; - import { createTranslator } from 'kolibri/utils/i18n'; import { coreStrings } from 'kolibri/uiText/commonCoreStrings'; -import AssessmentWrapper, { hintTranslator } from '../index.vue'; -const { completedLabel$ } = coreStrings; +import AssessmentWrapper from '../index.vue'; jest.mock('kolibri/composables/useUser'); const { - goal$, check$, next$, - inputAnswer$, - correct$, tryAgain$, + correct$, greatKeepGoing$, - itemError$, + hintUsed$, + inputAnswer$, + itemError$: itemErrorMessage$, tryDifferentQuestion$, + tryNextQuestion$, + goal$, } = createTranslator(AssessmentWrapper.name, AssessmentWrapper.$trs); +const { hint$, noMoreHint$ } = createTranslator('PerseusRendererIndex', { + hint: { message: 'Use a hint ({hintsLeft, number} left)' }, + noMoreHint: { message: 'No more hints' }, +}); + +const { completedLabel$ } = coreStrings; + const ASSESSMENT_IDS = ['item-1', 'item-2', 'item-3', 'item-4', 'item-5']; const DEFAULT_MASTERY_MODEL = { type: 'm_of_n', m: 3, n: 5 }; function buildProps(overrides = {}) { + const { + assessmentIds = ASSESSMENT_IDS, + randomize = false, + masteryModel = DEFAULT_MASTERY_MODEL, + ...rest + } = overrides; return { - files: [], - assessmentIds: ASSESSMENT_IDS, - randomize: false, - masteryModel: DEFAULT_MASTERY_MODEL, + contentNode: { + assessmentmetadata: { + assessment_item_ids: assessmentIds, + randomize, + mastery_model: masteryModel, + }, + }, pastattempts: [], mastered: false, totalattempts: 0, hasNextResource: false, - ...overrides, + ...rest, }; } @@ -48,8 +64,7 @@ function makeContentViewerStub({ checkAnswerFn, availableHints = 0, totalHints = return { name: 'ContentViewer', props: [ - 'lang', - 'files', + 'contentNode', 'extraFields', 'assessment', 'itemId', @@ -95,9 +110,11 @@ function renderComponent(props = {}, { stubs, ...restOptions } = {}) { const mergedProps = buildProps(props); return render(AssessmentWrapper, { props: mergedProps, - // ContentViewer is a complex renderer intentionally implemented as a stub - // to allow for testing AssessmentWrapper behavior in response to events - // emitted by ContentViewer + // The jest setup ships a minimal global ContentViewer stub, but AssessmentWrapper + // calls `this.$refs.contentViewer.checkAnswer()` and reads `availableHints`/`totalHints` + // from the viewer, which the global stub doesn't expose. Override it here. + // ContentViewer is globally registered, not imported, so jest.mock can't substitute + // it — a stub is the appropriate seam. // eslint-disable-next-line kolibri/tests-no-stubs stubs: { ContentViewer: DefaultStub, @@ -384,7 +401,7 @@ describe('AssessmentWrapper', () => { await fireEvent.click(screen.getByTestId('trigger-item-error')); await waitFor(() => { - expect(screen.getByText(itemError$())).toBeInTheDocument(); + expect(screen.getByText(itemErrorMessage$())).toBeInTheDocument(); expect(screen.getByText(tryDifferentQuestion$())).toBeInTheDocument(); }); }); @@ -394,7 +411,7 @@ describe('AssessmentWrapper', () => { await fireEvent.click(screen.getByTestId('trigger-item-error')); await waitFor(() => { - expect(getStatusText()).toBe('Try next question'); + expect(getStatusText()).toBe(tryNextQuestion$()); }); }); @@ -428,7 +445,7 @@ describe('AssessmentWrapper', () => { await fireEvent.click(screen.getByText(tryDifferentQuestion$())); await waitFor(() => { - expect(screen.queryByText(itemError$())).not.toBeInTheDocument(); + expect(screen.queryByText(itemErrorMessage$())).not.toBeInTheDocument(); expect(screen.getByText(check$())).toBeInTheDocument(); }); }); @@ -452,7 +469,7 @@ describe('AssessmentWrapper', () => { await fireEvent.click(screen.getByTestId('trigger-hint-taken')); await waitFor(() => { - expect(getStatusText()).toBe('Hint used'); + expect(getStatusText()).toBe(hintUsed$()); }); }); }); @@ -518,10 +535,8 @@ describe('AssessmentWrapper', () => { describe('hints UI', () => { it('does not show hint buttons when totalHints is 0', () => { renderComponent(); - expect( - screen.queryByText(hintTranslator.hint$({ hintsLeft: 1 }), { exact: false }), - ).not.toBeInTheDocument(); - expect(screen.queryByText(hintTranslator.noMoreHint$())).not.toBeInTheDocument(); + expect(screen.queryByText(hint$({ hintsLeft: 0 }))).not.toBeInTheDocument(); + expect(screen.queryByText(noMoreHint$())).not.toBeInTheDocument(); }); it('shows hint button with count after startTracking', async () => { @@ -531,7 +546,7 @@ describe('AssessmentWrapper', () => { await fireEvent.click(screen.getByTestId('trigger-start-tracking')); await waitFor(() => { - expect(screen.getByText(hintTranslator.hint$({ hintsLeft: 3 }))).toBeInTheDocument(); + expect(screen.getByText(hint$({ hintsLeft: 3 }))).toBeInTheDocument(); }); }); @@ -542,7 +557,7 @@ describe('AssessmentWrapper', () => { await fireEvent.click(screen.getByTestId('trigger-start-tracking')); await waitFor(() => { - expect(screen.getByText(hintTranslator.noMoreHint$())).toBeInTheDocument(); + expect(screen.getByText(noMoreHint$())).toBeInTheDocument(); }); }); @@ -553,10 +568,10 @@ describe('AssessmentWrapper', () => { await fireEvent.click(screen.getByTestId('trigger-start-tracking')); await waitFor(() => { - expect(screen.getByText(hintTranslator.hint$({ hintsLeft: 3 }))).toBeInTheDocument(); + expect(screen.getByText(hint$({ hintsLeft: 3 }))).toBeInTheDocument(); }); - await fireEvent.click(screen.getByText(hintTranslator.hint$({ hintsLeft: 3 }))); + await fireEvent.click(screen.getByText(hint$({ hintsLeft: 3 }))); await waitFor(() => { expect(emitted().updateInteraction).toBeTruthy(); @@ -572,22 +587,26 @@ describe('AssessmentWrapper', () => { assessmentIds: ['a', 'b'], masteryModel: { type: 'do_all' }, }); - expect(screen.getAllByText(goal$({ count: 2 })).length).toBeGreaterThanOrEqual(1); + const elements = screen.getAllByText(goal$({ count: 2 })); + expect(elements.length).toBeGreaterThanOrEqual(1); }); it('shows correct goal for num_correct_in_a_row_2', () => { renderComponent({ masteryModel: { type: 'num_correct_in_a_row_2' } }); - expect(screen.getAllByText(goal$({ count: 2 })).length).toBeGreaterThanOrEqual(1); + const elements = screen.getAllByText(goal$({ count: 2 })); + expect(elements.length).toBeGreaterThanOrEqual(1); }); it('shows correct goal for num_correct_in_a_row_3', () => { renderComponent({ masteryModel: { type: 'num_correct_in_a_row_3' } }); - expect(screen.getAllByText(goal$({ count: 3 })).length).toBeGreaterThanOrEqual(1); + const elements = screen.getAllByText(goal$({ count: 3 })); + expect(elements.length).toBeGreaterThanOrEqual(1); }); it('shows correct goal for num_correct_in_a_row_10', () => { renderComponent({ masteryModel: { type: 'num_correct_in_a_row_10' } }); - expect(screen.getAllByText(goal$({ count: 10 })).length).toBeGreaterThanOrEqual(1); + const elements = screen.getAllByText(goal$({ count: 10 })); + expect(elements.length).toBeGreaterThanOrEqual(1); }); it('renders the correct number of attempt placeholder slots for do_all', () => { diff --git a/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/index.vue b/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/index.vue index 0ae2b86ea19..bc7aed1ea5a 100644 --- a/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/index.vue +++ b/kolibri/plugins/learn/frontend/views/courses/AssessmentWrapper/index.vue @@ -54,8 +54,7 @@ > + import { computed } from 'vue'; import commonCoreStrings from 'kolibri/uiText/commonCoreStrings'; import { MasteryModelGenerators } from 'kolibri/constants'; - import { contentViewerProps } from 'kolibri/composables/useContentViewer'; + import { createTranslator } from 'kolibri/utils/i18n'; import shuffled from 'kolibri-common/utils/shuffled'; import UiAlert from 'kolibri-design-system/lib/keen/UiAlert'; import CoreInfoIcon from 'kolibri-common/components/labels/CoreInfoIcon'; - import { createTranslator } from 'kolibri/utils/i18n'; import useUser from 'kolibri/composables/useUser'; import { coursesStrings } from 'kolibri-common/strings/coursesStrings.js'; import ResourceLayout from '../../ResourceLayout/index.vue'; @@ -205,28 +204,31 @@ ResourceLayout, }, mixins: [commonCoreStrings], - setup() { + setup(props) { const { currentUserId } = useUser(); const { practiceAction$, nextLabel$ } = coursesStrings; + const assessmentIds = computed( + () => props.contentNode.assessmentmetadata.assessment_item_ids, + ); + const randomize = computed(() => props.contentNode.assessmentmetadata.randomize); + const masteryModel = computed(() => props.contentNode.assessmentmetadata.mastery_model); return { currentUserId, practiceAction$, nextLabel$, + assessmentIds, + randomize, + masteryModel, }; }, props: { - ...contentViewerProps, - assessmentIds: { - type: Array, - required: true, - }, - randomize: { - type: Boolean, + contentNode: { + type: Object, required: true, }, - masteryModel: { + extraFields: { type: Object, - required: true, + default: () => ({}), }, pastattempts: { type: Array, diff --git a/kolibri/plugins/media_player/frontend/utils/fileExtractors.js b/kolibri/plugins/media_player/frontend/utils/fileExtractors.js new file mode 100644 index 00000000000..6812219bc0e --- /dev/null +++ b/kolibri/plugins/media_player/frontend/utils/fileExtractors.js @@ -0,0 +1,67 @@ +/** + * File extractor functions for media elements (video/audio) + */ + +/** + * Generate file data object for media files + * @param {string} url - File URL + * @param {string} preset - Content preset + * @param {Object} options - Additional file options + * @returns {Object} File data object + */ +function _generateFileData(url, preset, options = {}) { + return { + storage_url: url, + preset, + available: true, + supplementary: false, + thumbnail: false, + priority: 1, + ...options, + }; +} + +/** + * Extract files from media elements (video/audio) + * @param {HTMLMediaElement} element - Media element + * @returns {Array} Array of file objects + */ +function extractMediaFiles(element) { + const files = []; + const preset = element.tagName.toLowerCase() === 'video' ? 'high_res_video' : 'audio'; + let sourceCount = 0; + + // Direct src attribute + if (element.src) { + files.push(_generateFileData(element.src, preset, { priority: 1 })); + sourceCount++; + } + + // and children + for (const child of element.children) { + const childTag = child.tagName?.toLowerCase(); + const childSrc = child.getAttribute('src'); + + if (childTag === 'source' && child.src) { + files.push(_generateFileData(child.src, preset, { priority: sourceCount + 1 })); + sourceCount++; + } else if (childTag === 'track' && childSrc && child.kind !== 'metadata') { + files.push( + _generateFileData(childSrc, 'video_subtitle', { + supplementary: true, + lang: child.srclang, + }), + ); + } + } + + return files; +} + +/** + * Media extractors for use with useContentViewer + */ +export default { + video: extractMediaFiles, + audio: extractMediaFiles, +}; diff --git a/kolibri/plugins/media_player/frontend/views/MediaPlayerIndex.vue b/kolibri/plugins/media_player/frontend/views/MediaPlayerIndex.vue index 9f4e9d9be22..3387a1f100c 100644 --- a/kolibri/plugins/media_player/frontend/views/MediaPlayerIndex.vue +++ b/kolibri/plugins/media_player/frontend/views/MediaPlayerIndex.vue @@ -96,11 +96,12 @@ import { mapActions, mapState, mapGetters } from 'vuex'; import videojs from 'video.js'; import throttle from 'lodash/throttle'; - import useContentViewer, { contentViewerProps } from 'kolibri/composables/useContentViewer'; + import useContentViewer from 'kolibri/composables/useContentViewer'; import { languageIdToCode } from 'kolibri/utils/i18n'; import commonCoreStrings from 'kolibri/uiText/commonCoreStrings'; import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow'; import useKResponsiveElement from 'kolibri-design-system/lib/composables/useKResponsiveElement'; + import customExtractors from '../utils/fileExtractors'; import Settings from '../utils/settings'; import { ReplayButton, ForwardButton } from './customButtons'; import MediaPlayerFullscreen from './MediaPlayerFullscreen'; @@ -129,24 +130,33 @@ const player = ref(null); const { windowIsSmall, windowIsPortrait } = useKResponsiveWindow(); const { elementWidth } = useKResponsiveElement(); - const { thumbnailFiles, supplementaryFiles, reportLoadingError } = useContentViewer( - props, - context, - { - defaultDuration: computed(() => player?.value?.duration()), - }, - ); + const { + files, + thumbnailFiles, + supplementaryFiles, + extraFields, + forceDurationBasedProgress, + durationBasedProgress, + reportLoadingError, + embedded, + } = useContentViewer(context, { + defaultDuration: computed(() => player?.value?.duration()), + customExtractors, + }); return { windowIsSmall, windowIsPortrait, elementWidth, + files, thumbnailFiles, supplementaryFiles, + extraFields, + forceDurationBasedProgress, + durationBasedProgress, reportLoadingError, player, }; }, - props: contentViewerProps, data: () => ({ dummyTime: 0, progressStartingPoint: 0, diff --git a/kolibri/plugins/media_player/kolibri_plugin.py b/kolibri/plugins/media_player/kolibri_plugin.py index 25fef8593bb..754534ada8d 100644 --- a/kolibri/plugins/media_player/kolibri_plugin.py +++ b/kolibri/plugins/media_player/kolibri_plugin.py @@ -17,3 +17,5 @@ class MediaPlayerAsset(content_hooks.ContentRendererHook): format_presets.VIDEO_HIGH_RES, format_presets.VIDEO_LOW_RES, ) + css_selectors = ("video", "audio") + allow_object_tag = True # Media player can handle object tags for audio/video diff --git a/kolibri/plugins/pdf_viewer/frontend/views/PdfRendererIndex.vue b/kolibri/plugins/pdf_viewer/frontend/views/PdfRendererIndex.vue index 94db28b279f..adddf3d4ff2 100644 --- a/kolibri/plugins/pdf_viewer/frontend/views/PdfRendererIndex.vue +++ b/kolibri/plugins/pdf_viewer/frontend/views/PdfRendererIndex.vue @@ -6,6 +6,8 @@ :class="{ 'pdf-controls-open': showControls, 'pdf-full-screen': isInFullscreen, + 'pdf-embedded': embedded, + 'pdf-mobile-embedded': mobileEmbedded, }" :style="{ backgroundColor: $themeTokens.text }" @changeFullscreen="isInFullscreen = $event" @@ -131,7 +133,7 @@ import commonCoreStrings from 'kolibri/uiText/commonCoreStrings'; import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow'; import CoreFullscreen from 'kolibri-common/components/CoreFullscreen'; - import useContentViewer, { contentViewerProps } from 'kolibri/composables/useContentViewer'; + import useContentViewer from 'kolibri/composables/useContentViewer'; import { ref, computed } from 'vue'; import '../utils/domPolyfills'; import { EventBus } from '../utils/event_utils'; @@ -160,7 +162,14 @@ const defaultDuration = computed(() => { return totalPages.value ? totalPages.value * 30 : null; }); - const { defaultFile, reportLoadingError } = useContentViewer(props, context, { + const { + defaultFile, + reportLoadingError, + embedded, + extraFields, + forceDurationBasedProgress, + durationBasedProgress, + } = useContentViewer(context, { defaultDuration, }); return { @@ -169,9 +178,12 @@ totalPages, defaultFile, reportLoadingError, + embedded, + extraFields, + forceDurationBasedProgress, + durationBasedProgress, }; }, - props: contentViewerProps, data: () => ({ loadingProgress: null, scale: null, @@ -210,10 +222,7 @@ return this.firstPageHeight * this.scale + MARGIN; }, savedLocation() { - if (this.extraFields && this.extraFields.contentState) { - return this.extraFields.contentState.savedLocation; - } - return 0; + return this.extraFields?.contentState?.savedLocation ?? 0; }, savedVisitedPages: { get() { diff --git a/kolibri/plugins/pdf_viewer/frontend/views/__tests__/PdfRendererIndex.spec.js b/kolibri/plugins/pdf_viewer/frontend/views/__tests__/PdfRendererIndex.spec.js index b06a9b9a91e..3cb51428935 100644 --- a/kolibri/plugins/pdf_viewer/frontend/views/__tests__/PdfRendererIndex.spec.js +++ b/kolibri/plugins/pdf_viewer/frontend/views/__tests__/PdfRendererIndex.spec.js @@ -1,15 +1,19 @@ import { render, screen, fireEvent } from '@testing-library/vue'; import { coreStrings } from 'kolibri/uiText/commonCoreStrings'; -import * as mockPDFJS from '../__mocks__/pdfjsMock'; +// eslint-disable-next-line import-x/named +import useContentViewer, { useContentViewerMock } from 'kolibri/composables/useContentViewer'; import PdfRendererIndex from '../PdfRendererIndex'; +import * as mockPDFJS from '../__mocks__/pdfjsMock'; -const { zoomIn$, zoomOut$ } = coreStrings; +jest.mock('kolibri/composables/useContentViewer'); jest.mock('kolibri/urls'); -jest.mock('pdfjs-dist/legacy/build/pdf', () => mockPDFJS); +jest.mock('pdfjs-dist/legacy/build/pdf', () => require('../__mocks__/pdfjsMock')); jest.mock('lodash/debounce', () => fn => fn); jest.mock('lodash/throttle', () => fn => fn); +const { zoomIn$, zoomOut$ } = coreStrings; + const DUMMY_PDF_URL = 'http://localhost:8000/test.pdf'; let vm = null; @@ -19,8 +23,7 @@ function makeWrapper(options = {}) { ...options, data: () => ({ defaultFile: { storage_url: DUMMY_PDF_URL }, - forceDurationBasedProgress: null, - ...(options.data ? options.data() : {}), + ...options.data, }), mixins: [ { @@ -123,11 +126,12 @@ describe('PdfRendererIndex', () => { it('should load the proper page when there is a saved location', async () => { const savedLocation = 0.2; mockPDFJS.PdfDocument.numPages = 10; - await loadPdfContainer({ - props: { - extraFields: { contentState: { savedLocation } }, - }, - }); + useContentViewer.mockImplementationOnce(() => + useContentViewerMock({ + extraFields: { contentState: { savedLocation, savedVisitedPages: {} } }, + }), + ); + await loadPdfContainer(); expect(mockPDFJS.PdfDocument.getPage).toHaveBeenCalledWith(3); }); }); diff --git a/kolibri/plugins/pdf_viewer/kolibri_plugin.py b/kolibri/plugins/pdf_viewer/kolibri_plugin.py index b5f8b8c019b..427afc9d11a 100644 --- a/kolibri/plugins/pdf_viewer/kolibri_plugin.py +++ b/kolibri/plugins/pdf_viewer/kolibri_plugin.py @@ -13,3 +13,4 @@ class DocumentPDFRenderPlugin(KolibriPluginBase): class DocumentPDFRenderAsset(content_hooks.ContentRendererHook): bundle_id = "main" presets = (format_presets.DOCUMENT,) + allow_object_tag = True diff --git a/kolibri/plugins/perseus_viewer/frontend/views/PerseusRendererIndex.vue b/kolibri/plugins/perseus_viewer/frontend/views/PerseusRendererIndex.vue index d85b4a0e2ac..b5b4f075ed0 100644 --- a/kolibri/plugins/perseus_viewer/frontend/views/PerseusRendererIndex.vue +++ b/kolibri/plugins/perseus_viewer/frontend/views/PerseusRendererIndex.vue @@ -37,7 +37,7 @@ import { Mapper, defaultFilePathMappers } from 'kolibri-zip/src/fileUtils'; import useKResponsiveWindow from 'kolibri-design-system/lib/composables/useKResponsiveWindow'; import { defer } from 'underscore'; - import useContentViewer, { contentViewerProps } from 'kolibri/composables/useContentViewer'; + import useContentViewer from 'kolibri/composables/useContentViewer'; import urls from 'kolibri/urls'; import { createElement as e } from 'react'; import { createPortal, render, unmountComponentAtNode } from 'react-dom'; @@ -249,14 +249,28 @@ name: 'PerseusRendererIndex', setup(props, context) { const { windowBreakpoint } = useKResponsiveWindow(); - const { defaultFile, contentDirection } = useContentViewer(props, context); + const { + defaultFile, + contentDirection, + itemData, + itemId, + answerState, + showCorrectAnswer, + interactive, + lang, + } = useContentViewer(context); return { windowBreakpoint, defaultFile, contentDirection, + itemData, + itemId, + answerState, + showCorrectAnswer, + interactive, + lang, }; }, - props: contentViewerProps, data: () => ({ // Is the perseus item loading? loading: true, diff --git a/kolibri/plugins/perseus_viewer/kolibri_plugin.py b/kolibri/plugins/perseus_viewer/kolibri_plugin.py index fd1cb02477f..d2d389fd1a9 100644 --- a/kolibri/plugins/perseus_viewer/kolibri_plugin.py +++ b/kolibri/plugins/perseus_viewer/kolibri_plugin.py @@ -13,3 +13,4 @@ class ExercisePerseusRenderPlugin(KolibriPluginBase): class ExercisePerseusRenderAsset(content_hooks.ContentRendererHook): bundle_id = "main" presets = (format_presets.EXERCISE,) + allow_object_tag = True diff --git a/kolibri/plugins/qti_viewer/frontend/components/QTIViewer.vue b/kolibri/plugins/qti_viewer/frontend/components/QTIViewer.vue index ca4db55c80d..edc80528044 100644 --- a/kolibri/plugins/qti_viewer/frontend/components/QTIViewer.vue +++ b/kolibri/plugins/qti_viewer/frontend/components/QTIViewer.vue @@ -15,7 +15,7 @@ import { computed, provide, ref, watch } from 'vue'; import logger from 'kolibri-logging'; - import useContentViewer, { contentViewerProps } from 'kolibri/composables/useContentViewer'; + import useContentViewer from 'kolibri/composables/useContentViewer'; import useQTIResource from '../composables/useQTIResource'; import { loadQTIPackage, parseXML } from '../utils/xml'; import AssessmentItem from './AssessmentItem.vue'; @@ -29,20 +29,28 @@ }, inheritAttrs: false, setup(props, context) { - const { defaultFile, reportLoadingError } = useContentViewer(props, context); + const { + defaultFile, + itemData, + itemId, + answerState, + userId, + interactive, + reportLoadingError, + } = useContentViewer(context); const packageLoading = ref(true); // Store resources by identifier const resourcesMap = ref({}); // Reactively get current resource based on itemId const currentResource = computed(() => { - return resourcesMap.value[props.itemId] || null; + return resourcesMap.value[itemId.value] || null; }); const resourceUrl = computed(() => currentResource.value?.href); // If itemData is provided, we only support injecting AssessmentItem XML const resourceType = computed(() => - props.itemData ? 'imsqti_item_xmlv3p0' : currentResource.value?.type, + itemData.value ? 'imsqti_item_xmlv3p0' : currentResource.value?.type, ); const { @@ -53,15 +61,15 @@ const xmlDoc = computed(() => { // If itemData is provided, use it directly - if (props.itemData) { - return parseXML(props.itemData); + if (itemData.value) { + return parseXML(itemData.value); } // Otherwise, use the resource XML document return resourceXmlDoc.value; }); const loading = computed(() => { - if (props.itemData) { + if (itemData.value) { return false; } return packageLoading.value || resourceLoading.value; @@ -127,8 +135,8 @@ 'QTI_CONTEXT', computed( () => - props.answerState?.QTI_CONTEXT ?? { - candidateIdentifier: props.userId, + answerState.value?.QTI_CONTEXT ?? { + candidateIdentifier: userId.value, testIdentifier: defaultFile?.checksum, environmentIdentifier: __version, }, @@ -137,11 +145,11 @@ provide( 'answerState', - computed(() => props.answerState || {}), + computed(() => answerState.value || {}), ); provide( 'interactive', - computed(() => props.interactive), + computed(() => interactive.value), ); return { @@ -154,7 +162,6 @@ checkAnswer, }; }, - props: contentViewerProps, }; diff --git a/kolibri/plugins/qti_viewer/kolibri_plugin.py b/kolibri/plugins/qti_viewer/kolibri_plugin.py index 04752095786..2bb400fdb17 100644 --- a/kolibri/plugins/qti_viewer/kolibri_plugin.py +++ b/kolibri/plugins/qti_viewer/kolibri_plugin.py @@ -13,3 +13,4 @@ class QTIViewerPlugin(KolibriPluginBase): class QTIViewerAsset(content_hooks.ContentRendererHook): bundle_id = "main" presets = (format_presets.QTI_ZIP,) + allow_object_tag = True diff --git a/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue b/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue index 2f1f5721da8..7fde821b2c7 100644 --- a/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue +++ b/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue @@ -24,29 +24,28 @@ + + + diff --git a/kolibri/plugins/media_player/frontend/views/AudioPlayerControls.vue b/kolibri/plugins/media_player/frontend/views/AudioPlayerControls.vue new file mode 100644 index 00000000000..885e258d9d6 --- /dev/null +++ b/kolibri/plugins/media_player/frontend/views/AudioPlayerControls.vue @@ -0,0 +1,427 @@ + + + + + + + diff --git a/kolibri/plugins/media_player/frontend/views/AudioStickyPlayer.vue b/kolibri/plugins/media_player/frontend/views/AudioStickyPlayer.vue new file mode 100644 index 00000000000..533a1fcd8ec --- /dev/null +++ b/kolibri/plugins/media_player/frontend/views/AudioStickyPlayer.vue @@ -0,0 +1,83 @@ + + + + + + + diff --git a/kolibri/plugins/media_player/frontend/views/MediaPlayerCaptions/SubtitlesMenuItem.vue b/kolibri/plugins/media_player/frontend/views/MediaPlayerCaptions/SubtitlesMenuItem.vue index 01efa0d20b8..fe5a5cabf82 100644 --- a/kolibri/plugins/media_player/frontend/views/MediaPlayerCaptions/SubtitlesMenuItem.vue +++ b/kolibri/plugins/media_player/frontend/views/MediaPlayerCaptions/SubtitlesMenuItem.vue @@ -17,17 +17,19 @@ - - - diff --git a/kolibri/plugins/media_player/frontend/views/MediaPlayerLanguages/LanguagesMenuItem.vue b/kolibri/plugins/media_player/frontend/views/MediaPlayerLanguages/LanguagesMenuItem.vue index d5330bdef9e..9b3e5fd8824 100644 --- a/kolibri/plugins/media_player/frontend/views/MediaPlayerLanguages/LanguagesMenuItem.vue +++ b/kolibri/plugins/media_player/frontend/views/MediaPlayerLanguages/LanguagesMenuItem.vue @@ -17,10 +17,21 @@ + + + diff --git a/kolibri/plugins/media_player/frontend/views/__tests__/AudioPlayer.spec.js b/kolibri/plugins/media_player/frontend/views/__tests__/AudioPlayer.spec.js new file mode 100644 index 00000000000..04b78eea8fc --- /dev/null +++ b/kolibri/plugins/media_player/frontend/views/__tests__/AudioPlayer.spec.js @@ -0,0 +1,182 @@ +import { render, screen, fireEvent } from '@testing-library/vue'; +import { nextTick, ref } from 'vue'; +import AudioPlayer from '../AudioPlayer'; +import mediaStrings from '../../utils/mediaStrings'; +/* eslint-disable import-x/named */ +import { + mockThumbnailFiles, + mockEmbedded, + mockCaptionTracks, + mockTranscript, + mockLoading, + mockIsPlaying, + mockInitPlayer, + mockToggleTranscript, + resetMocks, +} from '../../composables/useMediaPlayer'; +/* eslint-enable import-x/named */ + +const { play$, showTranscript$, hideTranscript$ } = mediaStrings; + +jest.mock('../../composables/useMediaPlayer'); + +const mockWindowIsSmall = ref(false); +jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => () => ({ + windowIsSmall: mockWindowIsSmall, +})); + +jest.mock('video.js', () => ({ + formatTime: seconds => { + const mins = Math.floor(seconds / 60); + const secs = Math.floor(seconds % 60); + return `${mins}:${secs < 10 ? '0' : ''}${secs}`; + }, +})); + +describe('AudioPlayer', () => { + let intersectionCallback; + const mockObserve = jest.fn(); + const mockDisconnect = jest.fn(); + + beforeAll(() => { + global.IntersectionObserver = jest.fn(callback => { + intersectionCallback = callback; + return { observe: mockObserve, disconnect: mockDisconnect }; + }); + }); + + beforeEach(() => { + resetMocks(); + mockObserve.mockClear(); + mockDisconnect.mockClear(); + }); + + describe('loading state', () => { + it('shows loader when loading', () => { + mockLoading.value = true; + render(AudioPlayer); + expect(screen.getByRole('progressbar')).toBeInTheDocument(); + }); + + it('hides controls when loading', () => { + mockLoading.value = true; + render(AudioPlayer); + expect(screen.queryByLabelText(play$())).not.toBeInTheDocument(); + }); + }); + + describe('audio sources', () => { + it('renders source elements for audio files', () => { + const { container } = render(AudioPlayer); + const sources = container.querySelectorAll('audio source'); + expect(sources).toHaveLength(2); + }); + + it('sets correct MIME type on source elements', () => { + const { container } = render(AudioPlayer); + const sources = container.querySelectorAll('audio source'); + expect(sources[0]).toHaveAttribute('type', 'audio/mpeg'); + expect(sources[1]).toHaveAttribute('type', 'audio/ogg'); + }); + }); + + describe('poster image', () => { + it('shows poster when thumbnail files exist', () => { + mockThumbnailFiles.value = [{ storage_url: '/poster.jpg' }]; + render(AudioPlayer); + const poster = screen.getByRole('img'); + expect(poster).toBeInTheDocument(); + expect(poster).toHaveAttribute('src', '/poster.jpg'); + }); + + it('does not render poster when no thumbnails', () => { + render(AudioPlayer); + expect(screen.queryByRole('img')).not.toBeInTheDocument(); + }); + }); + + describe('transcript', () => { + it('shows transcript toggle button when caption tracks exist', () => { + mockCaptionTracks.value = [{ id: 'en', lang: 'English' }]; + render(AudioPlayer); + expect(screen.getByRole('button', { name: showTranscript$() })).toBeInTheDocument(); + }); + + it('hides transcript toggle when no caption tracks', () => { + render(AudioPlayer); + expect(screen.queryByRole('button', { name: showTranscript$() })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: hideTranscript$() })).not.toBeInTheDocument(); + }); + + it('calls toggleTranscript when toggle button is clicked', async () => { + mockCaptionTracks.value = [{ id: 'en', lang: 'English' }]; + render(AudioPlayer); + await fireEvent.click(screen.getByRole('button', { name: showTranscript$() })); + expect(mockToggleTranscript).toHaveBeenCalledTimes(1); + }); + + it('shows "HIDE TRANSCRIPT" button when transcript is enabled', () => { + mockCaptionTracks.value = [{ id: 'en', lang: 'English' }]; + mockTranscript.value = true; + render(AudioPlayer); + expect(screen.getByRole('button', { name: hideTranscript$() })).toBeInTheDocument(); + }); + }); + + describe('sticky player', () => { + it('does not show sticky player when paused', () => { + const { container } = render(AudioPlayer); + expect(container.querySelector('.sticky-top, .sticky-bottom')).toBeNull(); + }); + + it('shows sticky player when playing and scrolled out of view', async () => { + mockIsPlaying.value = true; + const { container } = render(AudioPlayer); + intersectionCallback([{ isIntersecting: false }]); + await nextTick(); + expect( + container.querySelector('.sticky-top') || container.querySelector('.sticky-bottom'), + ).toBeTruthy(); + }); + + it('hides sticky player when scrolled back into view', async () => { + mockIsPlaying.value = true; + const { container } = render(AudioPlayer); + + intersectionCallback([{ isIntersecting: false }]); + await nextTick(); + expect( + container.querySelector('.sticky-top') || container.querySelector('.sticky-bottom'), + ).toBeTruthy(); + + intersectionCallback([{ isIntersecting: true }]); + await nextTick(); + expect(container.querySelector('.sticky-top, .sticky-bottom')).toBeNull(); + }); + }); + + describe('player initialization', () => { + it('calls initPlayer on mount', () => { + render(AudioPlayer); + expect(mockInitPlayer).toHaveBeenCalledTimes(1); + }); + + it('sets up IntersectionObserver on ready', () => { + render(AudioPlayer); + expect(mockObserve).toHaveBeenCalled(); + }); + }); + + describe('standalone vs embedded', () => { + it('applies standalone-wrapper class when not embedded', () => { + const { container } = render(AudioPlayer); + expect(container.querySelector('.standalone-wrapper')).toBeTruthy(); + }); + + it('does not apply standalone-wrapper when embedded', () => { + mockEmbedded.value = true; + const { container } = render(AudioPlayer); + expect(container.querySelector('.standalone-wrapper')).toBeNull(); + }); + }); +}); diff --git a/kolibri/plugins/media_player/frontend/views/__tests__/AudioPlayerControls.spec.js b/kolibri/plugins/media_player/frontend/views/__tests__/AudioPlayerControls.spec.js new file mode 100644 index 00000000000..e0ea353ff2f --- /dev/null +++ b/kolibri/plugins/media_player/frontend/views/__tests__/AudioPlayerControls.spec.js @@ -0,0 +1,166 @@ +import { render, screen, fireEvent } from '@testing-library/vue'; +import videojs from 'video.js'; +import AudioPlayerControls from '../AudioPlayerControls'; +import mediaStrings from '../../utils/mediaStrings'; +/* eslint-disable import-x/named */ +import { + mockCurrentTime, + mockDuration, + mockIsPlaying, + mockMuted, + mockPlaybackRate, + mockTogglePlay, + mockSeek, + mockRewind, + mockForward, + mockToggleMute, + mockSetPlaybackRate, + resetMocks, +} from '../../composables/useMediaPlayer'; +/* eslint-enable import-x/named */ + +jest.mock('../../composables/useMediaPlayer'); + +jest.mock('video.js', () => ({ + formatTime: seconds => { + const mins = Math.floor(seconds / 60); + const secs = Math.floor(seconds % 60); + return `${mins}:${secs < 10 ? '0' : ''}${secs}`; + }, +})); + +const { play$, pause$, replay$, forward$, mute$, unmute$, playbackRate$, progressBar$ } = + mediaStrings; + +describe('AudioPlayerControls', () => { + beforeEach(() => { + resetMocks(); + mockCurrentTime.value = 30; + }); + + describe('transport controls', () => { + it('renders play button when paused', () => { + render(AudioPlayerControls); + expect(screen.getByLabelText(play$())).toBeInTheDocument(); + }); + + it('renders pause button when playing', () => { + mockIsPlaying.value = true; + render(AudioPlayerControls); + expect(screen.getByLabelText(pause$())).toBeInTheDocument(); + }); + + it('calls togglePlay when play button is clicked', async () => { + render(AudioPlayerControls); + await fireEvent.click(screen.getByLabelText(play$())); + expect(mockTogglePlay).toHaveBeenCalledTimes(1); + }); + + it('renders rewind and forward buttons', () => { + render(AudioPlayerControls); + expect(screen.getByLabelText(replay$())).toBeInTheDocument(); + expect(screen.getByLabelText(forward$())).toBeInTheDocument(); + }); + + it('calls rewind when rewind button is clicked', async () => { + render(AudioPlayerControls); + await fireEvent.click(screen.getByLabelText(replay$())); + expect(mockRewind).toHaveBeenCalledTimes(1); + }); + + it('calls forward when forward button is clicked', async () => { + render(AudioPlayerControls); + await fireEvent.click(screen.getByLabelText(forward$())); + expect(mockForward).toHaveBeenCalledTimes(1); + }); + }); + + describe('progress display', () => { + it('displays formatted current time', () => { + render(AudioPlayerControls); + expect(screen.getByText(videojs.formatTime(mockCurrentTime.value))).toBeInTheDocument(); + }); + + it('displays formatted duration', () => { + render(AudioPlayerControls); + expect(screen.getByText(videojs.formatTime(mockDuration.value))).toBeInTheDocument(); + }); + + it('renders a progress slider with correct ARIA attributes', () => { + render(AudioPlayerControls); + const slider = screen.getByRole('slider', { name: progressBar$() }); + expect(slider).toBeInTheDocument(); + expect(slider).toHaveAttribute('aria-valuemin', '0'); + expect(slider).toHaveAttribute('aria-valuemax', String(mockDuration.value)); + expect(slider).toHaveAttribute('aria-valuenow', String(mockCurrentTime.value)); + }); + }); + + describe('secondary controls', () => { + it('renders mute button when not muted', () => { + render(AudioPlayerControls); + expect(screen.getByLabelText(mute$())).toBeInTheDocument(); + }); + + it('renders unmute button when muted', () => { + mockMuted.value = true; + render(AudioPlayerControls); + expect(screen.getByLabelText(unmute$())).toBeInTheDocument(); + }); + + it('calls toggleMute when mute button is clicked', async () => { + render(AudioPlayerControls); + await fireEvent.click(screen.getByLabelText(mute$())); + expect(mockToggleMute).toHaveBeenCalledTimes(1); + }); + + it('displays playback rate label', () => { + render(AudioPlayerControls); + const rateLabel = `${mockPlaybackRate.value}X`; + expect(screen.getByText(rateLabel)).toBeInTheDocument(); + }); + + it('cycles playback rate when rate button is clicked', async () => { + render(AudioPlayerControls); + await fireEvent.click(screen.getByLabelText(playbackRate$())); + expect(mockSetPlaybackRate).toHaveBeenCalledWith(1.25); + }); + + it('wraps back to 0.5X after 2X', async () => { + mockPlaybackRate.value = 2.0; + render(AudioPlayerControls); + await fireEvent.click(screen.getByLabelText(playbackRate$())); + expect(mockSetPlaybackRate).toHaveBeenCalledWith(0.5); + }); + }); + + describe('keyboard navigation', () => { + it('seeks forward on ArrowRight key', async () => { + render(AudioPlayerControls); + const slider = screen.getByRole('slider', { name: progressBar$() }); + await fireEvent.keyDown(slider, { key: 'ArrowRight' }); + expect(mockForward).toHaveBeenCalledWith(5); + }); + + it('seeks backward on ArrowLeft key', async () => { + render(AudioPlayerControls); + const slider = screen.getByRole('slider', { name: progressBar$() }); + await fireEvent.keyDown(slider, { key: 'ArrowLeft' }); + expect(mockRewind).toHaveBeenCalledWith(5); + }); + + it('seeks to start on Home key', async () => { + render(AudioPlayerControls); + const slider = screen.getByRole('slider', { name: progressBar$() }); + await fireEvent.keyDown(slider, { key: 'Home' }); + expect(mockSeek).toHaveBeenCalledWith(0); + }); + + it('seeks to end on End key', async () => { + render(AudioPlayerControls); + const slider = screen.getByRole('slider', { name: progressBar$() }); + await fireEvent.keyDown(slider, { key: 'End' }); + expect(mockSeek).toHaveBeenCalledWith(mockDuration.value); + }); + }); +}); diff --git a/kolibri/plugins/media_player/frontend/views/__tests__/AudioStickyPlayer.spec.js b/kolibri/plugins/media_player/frontend/views/__tests__/AudioStickyPlayer.spec.js new file mode 100644 index 00000000000..c5e166d5391 --- /dev/null +++ b/kolibri/plugins/media_player/frontend/views/__tests__/AudioStickyPlayer.spec.js @@ -0,0 +1,155 @@ +import { render, screen, fireEvent } from '@testing-library/vue'; +import { ref } from 'vue'; +import videojs from 'video.js'; +import AudioStickyPlayer from '../AudioStickyPlayer'; +import mediaStrings from '../../utils/mediaStrings'; +/* eslint-disable import-x/named */ +import { + mockCurrentTime, + mockDuration, + mockIsPlaying, + mockTogglePlay, + mockRewind, + mockForward, + mockToggleMute, + mockSetPlaybackRate, + resetMocks, +} from '../../composables/useMediaPlayer'; +/* eslint-enable import-x/named */ + +jest.mock('../../composables/useMediaPlayer'); + +const mockWindowIsSmall = ref(false); +jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => () => ({ + windowIsSmall: mockWindowIsSmall, +})); + +jest.mock('video.js', () => ({ + formatTime: seconds => { + const mins = Math.floor(seconds / 60); + const secs = Math.floor(seconds % 60); + return `${mins}:${secs < 10 ? '0' : ''}${secs}`; + }, +})); + +const { pause$, replay$, forward$, mute$, playbackRate$, progressBar$ } = mediaStrings; + +describe('AudioStickyPlayer', () => { + beforeEach(() => { + resetMocks(); + mockCurrentTime.value = 45; + mockDuration.value = 300; + mockIsPlaying.value = true; + mockWindowIsSmall.value = false; + }); + + describe('desktop layout', () => { + it('renders all transport buttons', () => { + render(AudioStickyPlayer); + expect(screen.getByLabelText(pause$())).toBeInTheDocument(); + expect(screen.getByLabelText(replay$())).toBeInTheDocument(); + expect(screen.getByLabelText(forward$())).toBeInTheDocument(); + }); + + it('displays current time and duration', () => { + render(AudioStickyPlayer); + expect(screen.getByText(videojs.formatTime(mockCurrentTime.value))).toBeInTheDocument(); + expect(screen.getByText(videojs.formatTime(mockDuration.value))).toBeInTheDocument(); + }); + + it('renders progress slider with ARIA attributes', () => { + render(AudioStickyPlayer); + const slider = screen.getByRole('slider', { name: progressBar$() }); + expect(slider).toBeInTheDocument(); + expect(slider).toHaveAttribute('aria-valuenow', String(mockCurrentTime.value)); + expect(slider).toHaveAttribute('aria-valuemax', String(mockDuration.value)); + }); + + it('renders playback rate and volume buttons', () => { + render(AudioStickyPlayer); + expect(screen.getByLabelText(playbackRate$())).toBeInTheDocument(); + expect(screen.getByLabelText(mute$())).toBeInTheDocument(); + }); + + it('applies sticky-top class on desktop', () => { + const { container } = render(AudioStickyPlayer); + expect(container.querySelector('.sticky-top')).toBeTruthy(); + expect(container.querySelector('.sticky-bottom')).toBeNull(); + }); + }); + + describe('mobile layout', () => { + beforeEach(() => { + mockWindowIsSmall.value = true; + }); + + it('applies sticky-bottom class on mobile', () => { + const { container } = render(AudioStickyPlayer); + expect(container.querySelector('.sticky-bottom')).toBeTruthy(); + expect(container.querySelector('.sticky-top')).toBeNull(); + }); + + it('renders two-row layout on mobile', () => { + const { container } = render(AudioStickyPlayer); + expect(container.querySelector('.rows-2')).toBeTruthy(); + expect(container.querySelector('.rows-1')).toBeNull(); + }); + + it('renders all controls in mobile layout', () => { + render(AudioStickyPlayer); + expect(screen.getByLabelText(pause$())).toBeInTheDocument(); + expect(screen.getByLabelText(replay$())).toBeInTheDocument(); + expect(screen.getByLabelText(forward$())).toBeInTheDocument(); + expect(screen.getByLabelText(playbackRate$())).toBeInTheDocument(); + expect(screen.getByLabelText(mute$())).toBeInTheDocument(); + }); + }); + + describe('interactions', () => { + it('calls togglePlay when pause button is clicked', async () => { + render(AudioStickyPlayer); + await fireEvent.click(screen.getByLabelText(pause$())); + expect(mockTogglePlay).toHaveBeenCalledTimes(1); + }); + + it('calls rewind when rewind button is clicked', async () => { + render(AudioStickyPlayer); + await fireEvent.click(screen.getByLabelText(replay$())); + expect(mockRewind).toHaveBeenCalledTimes(1); + }); + + it('calls forward when forward button is clicked', async () => { + render(AudioStickyPlayer); + await fireEvent.click(screen.getByLabelText(forward$())); + expect(mockForward).toHaveBeenCalledTimes(1); + }); + + it('cycles playback rate on click', async () => { + render(AudioStickyPlayer); + await fireEvent.click(screen.getByLabelText(playbackRate$())); + expect(mockSetPlaybackRate).toHaveBeenCalledWith(1.25); + }); + + it('calls toggleMute when volume button is clicked', async () => { + render(AudioStickyPlayer); + await fireEvent.click(screen.getByLabelText(mute$())); + expect(mockToggleMute).toHaveBeenCalledTimes(1); + }); + }); + + describe('keyboard navigation on progress bar', () => { + it('seeks forward on ArrowRight key', async () => { + render(AudioStickyPlayer); + const slider = screen.getByRole('slider', { name: progressBar$() }); + await fireEvent.keyDown(slider, { key: 'ArrowRight' }); + expect(mockForward).toHaveBeenCalledWith(5); + }); + + it('seeks backward on ArrowLeft key', async () => { + render(AudioStickyPlayer); + const slider = screen.getByRole('slider', { name: progressBar$() }); + await fireEvent.keyDown(slider, { key: 'ArrowLeft' }); + expect(mockRewind).toHaveBeenCalledWith(5); + }); + }); +}); diff --git a/kolibri/plugins/media_player/frontend/views/__tests__/MediaPlayerIndex.spec.js b/kolibri/plugins/media_player/frontend/views/__tests__/MediaPlayerIndex.spec.js deleted file mode 100644 index a2b24b34a0a..00000000000 --- a/kolibri/plugins/media_player/frontend/views/__tests__/MediaPlayerIndex.spec.js +++ /dev/null @@ -1,23 +0,0 @@ -import MediaPlayerIndex from '../MediaPlayerIndex'; - -const { methods } = MediaPlayerIndex; - -describe('updateProgress', () => { - let context = {}; - - beforeEach(() => { - context = { - forceDurationBasedProgress: null, - $emit: jest.fn(), - durationBasedProgress: 0.1, - }; - }); - - it('should be able to calculate progress using time-based tracking', () => { - context.forceDurationBasedProgress = true; - methods.recordProgress.call(context); - - expect(context.$emit.mock.calls[0][0]).toBe('updateProgress'); - expect(context.$emit.mock.calls[0][1]).toBe(0.1); - }); -}); diff --git a/kolibri/plugins/media_player/frontend/views/customButtons.js b/kolibri/plugins/media_player/frontend/views/customButtons.js index bead6b81813..ab4e2f5b896 100644 --- a/kolibri/plugins/media_player/frontend/views/customButtons.js +++ b/kolibri/plugins/media_player/frontend/views/customButtons.js @@ -25,3 +25,14 @@ export class ForwardButton extends videojsButton { } ForwardButton.prototype.controlText_ = 'Forward'; + +const PlayToggle = videojs.getComponent('PlayToggle'); + +// Centered overlay play/pause toggle — extends PlayToggle so it inherits the +// play/pause icon switching tied to player state, but adds a distinct class +// so it can be styled separately from the small toggle in the control bar. +export class BigPlayToggle extends PlayToggle { + buildCSSClass() { + return `vjs-big-play-toggle ${super.buildCSSClass()}`; + } +} diff --git a/kolibri/plugins/media_player/package.json b/kolibri/plugins/media_player/package.json index b497dd26dc5..10d4a788f9d 100644 --- a/kolibri/plugins/media_player/package.json +++ b/kolibri/plugins/media_player/package.json @@ -5,6 +5,7 @@ "version": "0.0.1", "dependencies": { "@babel/runtime": "^7.29.2", + "@testing-library/vue": "catalog:", "@vueuse/core": "catalog:", "frame-throttle": "^3.0.0", "global": "^4.4.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e5539f77334..61622cf8027 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -540,6 +540,9 @@ importers: '@babel/runtime': specifier: ^7.29.2 version: 7.29.2 + '@testing-library/vue': + specifier: 'catalog:' + version: 5.9.0(vue-template-compiler@2.7.16)(vue@2.7.16) '@vueuse/core': specifier: 'catalog:' version: 11.3.0(vue@2.7.16) @@ -1826,10 +1829,6 @@ packages: '@babel/code-frame@7.12.11': resolution: {integrity: sha512-Zt1yodBx1UcyiePMSkWnU4hPqhwq7hGi2nFL1LeA3EUl+q2LQx16MISgJ0+z7dnmgvP9QtIleuETGOiOH1RcIw==} - '@babel/code-frame@7.28.6': - resolution: {integrity: sha512-JYgintcMjRiCvS8mMECzaEn+m3PfoQiyqukOMCCVQtoJGYJw8j/8LBJEiqkHLkfwCcs74E3pbAUFNg7d9VNJ+Q==} - engines: {node: '>=6.9.0'} - '@babel/code-frame@7.29.0': resolution: {integrity: sha512-9NhCeYjq9+3uxgdtp20LSiJXJvN0FeCtNGpJxuMFZ1Kv3cWUNb6DOhJwUvcVCzKGR66cw4njwM6hrJLqgOwbcw==} engines: {node: '>=6.9.0'} @@ -9659,12 +9658,6 @@ snapshots: dependencies: '@babel/highlight': 7.25.9 - '@babel/code-frame@7.28.6': - dependencies: - '@babel/helper-validator-identifier': 7.28.5 - js-tokens: 4.0.0 - picocolors: 1.1.1 - '@babel/code-frame@7.29.0': dependencies: '@babel/helper-validator-identifier': 7.28.5 @@ -11306,7 +11299,7 @@ snapshots: '@babel/traverse@7.28.6': dependencies: - '@babel/code-frame': 7.28.6 + '@babel/code-frame': 7.29.0 '@babel/generator': 7.28.6 '@babel/helper-globals': 7.28.0 '@babel/parser': 7.28.6 @@ -16289,7 +16282,7 @@ snapshots: jest-message-util@30.2.0: dependencies: - '@babel/code-frame': 7.28.6 + '@babel/code-frame': 7.29.0 '@jest/types': 30.2.0 '@types/stack-utils': 2.0.3 chalk: 4.1.2 @@ -17388,7 +17381,7 @@ snapshots: parse-json@5.2.0: dependencies: - '@babel/code-frame': 7.28.6 + '@babel/code-frame': 7.29.0 error-ex: 1.3.4 json-parse-even-better-errors: 2.3.1 lines-and-columns: 1.2.4 From c49ad31a7876dc320694c1895aee662761a66f1f Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 8 Apr 2026 16:42:22 -0700 Subject: [PATCH 3/5] Add viewer ID tracking for embedded content progress aggregation ContentViewer now generates unique viewer IDs and appends them to all interaction events, allowing parent components to track which embedded viewer emitted each event. SafeHtml5RendererIndex uses this to aggregate progress from multiple embedded viewers (e.g., videos in HTML5 content), combining scroll-based progress with embedded viewer progress using dynamic weighting. --- .../frontend/views/SafeHtml5RendererIndex.vue | 104 ++++++++- .../__tests__/SafeHtml5RendererIndex.spec.js | 29 ++- ...feHtml5RendererProgressAggregation.spec.js | 213 ++++++++++++++++++ pnpm-lock.yaml | 2 +- 4 files changed, 334 insertions(+), 14 deletions(-) create mode 100644 kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererProgressAggregation.spec.js diff --git a/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue b/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue index 7fde821b2c7..9b835cddfb0 100644 --- a/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue +++ b/kolibri/plugins/safe_html5_viewer/frontend/views/SafeHtml5RendererIndex.vue @@ -14,7 +14,14 @@ role="region" :aria-label="$tr('articleContent')" > - + @@ -52,12 +59,50 @@ html: null, scrollBasedProgress: 0, debouncedHandleScroll: null, + // Track embedded viewers for progress aggregation + // Using object instead of Map for Vue 2.7 reactivity + // Structure: { viewerId: { progress: number, complete: boolean } } + embeddedViewers: {}, + // Guard to prevent emitting 'finished' multiple times + hasEmittedFinished: false, }; }, computed: { entry() { return (this.options && this.options.entry) || 'index.html'; }, + // Count of registered embedded viewers + viewerCount() { + return Object.keys(this.embeddedViewers).length; + }, + // Aggregated progress using dynamic weighting + // If no embedded viewers: progress = scrollBasedProgress + // If viewers exist: progress = (scrollBasedProgress + avgViewerProgress) / 2 + aggregatedProgress() { + if (this.viewerCount === 0) { + return this.scrollBasedProgress; + } + + let totalViewerProgress = 0; + for (const viewer of Object.values(this.embeddedViewers)) { + totalViewerProgress += viewer.progress; + } + const avgViewerProgress = totalViewerProgress / this.viewerCount; + + // Dynamic weighting: 50% scroll, 50% viewers + return (this.scrollBasedProgress + avgViewerProgress) / 2; + }, + // Whether all sources are complete + allSourcesComplete() { + // All viewers must be complete + for (const viewer of Object.values(this.embeddedViewers)) { + if (!viewer.complete) { + return false; + } + } + + return true; + }, }, async created() { const storageUrl = this.defaultFile.storage_url; @@ -100,18 +145,67 @@ } }); }, + + // Handle startTracking from embedded viewers + handleViewerStartTracking(viewerId) { + if (viewerId && !this.embeddedViewers[viewerId]) { + this.$set(this.embeddedViewers, viewerId, { progress: 0, complete: false }); + } + }, + + // Handle stopTracking from embedded viewers + handleViewerStopTracking(viewerId) { + if (viewerId) { + this.$delete(this.embeddedViewers, viewerId); + } + }, + + // Handle updateProgress from embedded viewers + handleViewerUpdateProgress(progress, viewerId) { + if (viewerId && this.embeddedViewers[viewerId]) { + this.$set(this.embeddedViewers, viewerId, { + ...this.embeddedViewers[viewerId], + progress: Math.min(1, Math.max(0, progress)), + }); + } + }, + + // Handle addProgress from embedded viewers + handleViewerAddProgress(delta, viewerId) { + if (viewerId) { + const viewer = this.embeddedViewers[viewerId]; + if (viewer) { + const newProgress = Math.min(1, Math.max(0, viewer.progress + delta)); + this.$set(this.embeddedViewers, viewerId, { + ...viewer, + progress: newProgress, + }); + } + } + }, + + // Handle finished from embedded viewers + handleViewerFinished(viewerId) { + if (viewerId && this.embeddedViewers[viewerId]) { + this.$set(this.embeddedViewers, viewerId, { + progress: 1, + complete: true, + }); + } + }, + recordProgress() { let progress; if (this.forceDurationBasedProgress) { progress = this.durationBasedProgress; } else { - // Use scroll events to track progress - progress = this.scrollBasedProgress; + // Use aggregated progress from scroll + embedded viewers + progress = this.aggregatedProgress; } this.$emit('updateProgress', progress); - - if (progress >= 1) { + if (progress >= 1 && this.allSourcesComplete && !this.hasEmittedFinished) { this.$emit('finished'); + this.hasEmittedFinished = true; } this.pollProgress(); }, diff --git a/kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererIndex.spec.js b/kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererIndex.spec.js index 58e0312b396..a1d1249f732 100644 --- a/kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererIndex.spec.js +++ b/kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererIndex.spec.js @@ -1,7 +1,16 @@ import { render, screen, waitFor } from '@testing-library/vue'; import { createTranslator } from 'kolibri/utils/i18n'; +// eslint-disable-next-line import-x/named +import useContentViewer, { useContentViewerMock } from 'kolibri/composables/useContentViewer'; import SafeHtml5RendererIndex from '../SafeHtml5RendererIndex.vue'; +// Mock kolibri to prevent initialization side effects +jest.mock('kolibri', () => ({ + canHandleElement: jest.fn(() => false), +})); + +jest.mock('kolibri/composables/useContentViewer'); + const { articleContent$ } = createTranslator( SafeHtml5RendererIndex.name, SafeHtml5RendererIndex.$trs, @@ -31,13 +40,9 @@ jest.mock('kolibri-zip', () => { })); }); -const DUMMY_HTML5_URL = 'mock://test.html'; const renderComponent = (dataOverrides = {}) => { return render(SafeHtml5RendererIndex, { - data: () => ({ - defaultFile: { storage_url: DUMMY_HTML5_URL }, - ...dataOverrides, - }), + data: () => dataOverrides, }); }; @@ -57,6 +62,12 @@ async function setupTableContainer(scrollWidth, clientWidth) { } describe('SafeHtml5RendererIndex', () => { + beforeEach(() => { + useContentViewer.mockImplementation(() => + useContentViewerMock({ defaultFile: { storage_url: 'mock://test.html' } }), + ); + }); + describe('first render', () => { it('smoke test', async () => { renderComponent(); @@ -79,9 +90,11 @@ describe('SafeHtml5RendererIndex', () => { renderComponent(); await waitFor(() => { expect(screen.getByLabelText(articleContent$())).toBeInTheDocument(); - expect(screen.getByText(HEADING)).toBeInTheDocument(); - expect(screen.getByText(TABLE_CAPTION)).toBeInTheDocument(); - CELLS.forEach(cell => expect(screen.getByText(cell)).toBeInTheDocument()); + expect(screen.getByRole('heading', { name: HEADING, level: 1 })).toBeInTheDocument(); + expect(screen.getByText(TABLE_CAPTION).tagName).toBe('CAPTION'); + CELLS.forEach(cell => { + expect(screen.getByRole('cell', { name: cell })).toBeInTheDocument(); + }); }); }); }); diff --git a/kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererProgressAggregation.spec.js b/kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererProgressAggregation.spec.js new file mode 100644 index 00000000000..c8d98768361 --- /dev/null +++ b/kolibri/plugins/safe_html5_viewer/frontend/views/__tests__/SafeHtml5RendererProgressAggregation.spec.js @@ -0,0 +1,213 @@ +import { render, screen, waitFor } from '@testing-library/vue'; +import { createTranslator } from 'kolibri/utils/i18n'; +// eslint-disable-next-line import-x/named +import useContentViewer, { useContentViewerMock } from 'kolibri/composables/useContentViewer'; +import SafeHtml5RendererIndex from '../SafeHtml5RendererIndex.vue'; + +const { articleContent$ } = createTranslator( + SafeHtml5RendererIndex.name, + SafeHtml5RendererIndex.$trs, +); + +let mockEmitFromSafeHTML; + +jest.mock('kolibri', () => ({ + canHandleElement: jest.fn(() => false), +})); + +jest.mock('kolibri/composables/useContentViewer'); + +// Mock SafeHTML to render html content and provide controllable event emission +// for testing how the parent aggregates progress from embedded viewers. +jest.mock('kolibri-common/components/SafeHTML', () => ({ + createSafeHTML: () => ({ + name: 'SafeHTML', + props: { html: String }, + created() { + mockEmitFromSafeHTML = (event, ...args) => this.$emit(event, ...args); + }, + render(h) { + return h('div', { domProps: { innerHTML: this.html || '' } }); + }, + }), +})); + +jest.mock('kolibri-common/components/SafeHTML/style.scss', () => ({})); +jest.mock('kolibri-zip', () => { + return jest.fn().mockImplementation(() => ({ + file: jest.fn().mockResolvedValue({ + toString: () => '

Mocked HTML content

', + }), + })); +}); + +const renderComponent = () => { + return render(SafeHtml5RendererIndex); +}; + +describe('SafeHtml5RendererIndex progress aggregation', () => { + beforeEach(() => { + useContentViewer.mockImplementation(() => + useContentViewerMock({ defaultFile: { storage_url: 'mock://test.html' } }), + ); + jest.useFakeTimers(); + mockEmitFromSafeHTML = null; + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + async function renderAndLoad() { + const result = renderComponent(); + await waitFor(() => { + expect(screen.getByLabelText(articleContent$())).toBeInTheDocument(); + }); + return result; + } + + function getLastEmittedProgress(emitted) { + jest.advanceTimersByTime(5000); + const events = emitted().updateProgress; + return events[events.length - 1][0]; + } + + // In jsdom, scrollHeight and clientHeight are both 0, so maxScroll = 0 + // and handleScroll sets scrollBasedProgress = 1 (content considered fully read). + function simulateFullScroll() { + const wrapper = document.querySelector('[data-testid="safe-html-wrapper"]'); + wrapper.dispatchEvent(new Event('scroll')); + jest.advanceTimersByTime(150); // flush debounce + } + + it('reports scroll-based progress when no embedded viewers exist', async () => { + const { emitted } = await renderAndLoad(); + expect(getLastEmittedProgress(emitted)).toBe(0); + }); + + it('averages scroll and viewer progress with one viewer', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', 0.5, 'viewer-1'); + // scroll=0, viewer=0.5, aggregated=(0+0.5)/2=0.25 + expect(getLastEmittedProgress(emitted)).toBe(0.25); + }); + + it('averages scroll and average viewer progress with multiple viewers', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('startTracking', 'viewer-2'); + mockEmitFromSafeHTML('updateProgress', 0.25, 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', 0.75, 'viewer-2'); + // scroll=0, viewer avg=(0.25+0.75)/2=0.5, aggregated=(0+0.5)/2=0.25 + expect(getLastEmittedProgress(emitted)).toBe(0.25); + }); + + it('accumulates delta progress via addProgress', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('addProgress', 0.25, 'viewer-1'); + mockEmitFromSafeHTML('addProgress', 0.25, 'viewer-1'); + // viewer=0.5, scroll=0, aggregated=(0+0.5)/2=0.25 + expect(getLastEmittedProgress(emitted)).toBe(0.25); + }); + + it('clamps negative progress to 0', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', -0.5, 'viewer-1'); + // clamped to 0, aggregated=(0+0)/2=0 + expect(getLastEmittedProgress(emitted)).toBe(0); + }); + + it('clamps progress above 1 to 1', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', 1.5, 'viewer-1'); + // clamped to 1, aggregated=(0+1)/2=0.5 + expect(getLastEmittedProgress(emitted)).toBe(0.5); + }); + + it('reverts to scroll-only progress after viewer unregisters', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', 0.8, 'viewer-1'); + mockEmitFromSafeHTML('stopTracking', 'viewer-1'); + // no viewers left, progress=scrollBasedProgress=0 + expect(getLastEmittedProgress(emitted)).toBe(0); + }); + + it('does not re-register an already registered viewer', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', 0.5, 'viewer-1'); + // Re-registering should not reset progress + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + // Still one viewer at 0.5, aggregated=(0+0.5)/2=0.25 + expect(getLastEmittedProgress(emitted)).toBe(0.25); + }); + + it('ignores events without a viewerId', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', null); + mockEmitFromSafeHTML('updateProgress', 0.5, null); + mockEmitFromSafeHTML('addProgress', 0.5, null); + mockEmitFromSafeHTML('finished', null); + mockEmitFromSafeHTML('stopTracking', null); + // No viewers registered, progress=scroll=0 + expect(getLastEmittedProgress(emitted)).toBe(0); + }); + + it('ignores updates to unregistered viewers', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('updateProgress', 0.5, 'unknown'); + mockEmitFromSafeHTML('addProgress', 0.5, 'unknown'); + mockEmitFromSafeHTML('finished', 'unknown'); + // No viewers, progress=scroll=0 + expect(getLastEmittedProgress(emitted)).toBe(0); + }); + + it('does not emit finished when progress is below 1', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', 0.5, 'viewer-1'); + jest.advanceTimersByTime(5000); + expect(emitted().finished).toBeUndefined(); + }); + + it('does not emit finished when viewers are incomplete', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('updateProgress', 0.5, 'viewer-1'); + simulateFullScroll(); + // scroll=1, viewer=0.5, aggregated=(1+0.5)/2=0.75 and viewer not complete + expect(emitted().finished).toBeUndefined(); + }); + + it('emits finished when scroll is complete and no embedded viewers exist', async () => { + const { emitted } = await renderAndLoad(); + simulateFullScroll(); + // scroll=1, no viewers, aggregated=1, allSourcesComplete=true + expect(emitted().finished).toHaveLength(1); + }); + + it('emits finished when scroll is complete and all viewers are finished', async () => { + const { emitted } = await renderAndLoad(); + mockEmitFromSafeHTML('startTracking', 'viewer-1'); + mockEmitFromSafeHTML('startTracking', 'viewer-2'); + mockEmitFromSafeHTML('finished', 'viewer-1'); + mockEmitFromSafeHTML('finished', 'viewer-2'); + simulateFullScroll(); + // scroll=1, both viewers complete at progress=1 + // aggregated=(1+1)/2=1, allSourcesComplete=true + expect(emitted().finished).toHaveLength(1); + }); + + it('emits finished only once across multiple recordProgress calls', async () => { + const { emitted } = await renderAndLoad(); + simulateFullScroll(); // triggers recordProgress via handleScroll + jest.advanceTimersByTime(5000); // triggers another recordProgress via poll + jest.advanceTimersByTime(5000); // and another + expect(emitted().finished).toHaveLength(1); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 61622cf8027..6a828112b5e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -15016,7 +15016,7 @@ snapshots: json-stable-stringify-without-jsonify: 1.0.1 levn: 0.4.1 lodash.merge: 4.6.2 - minimatch: 3.1.2 + minimatch: 3.1.5 natural-compare: 1.4.0 optionator: 0.9.4 progress: 2.0.3 From 19241459af9a868fd2b33c06fa917738a151402d Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Wed, 8 Apr 2026 16:40:58 -0700 Subject: [PATCH 4/5] Add shared ViewerToolbar and integrate across all viewers --- .../frontend/views/BloomPubRendererIndex.vue | 84 ++--- .../frontend/views/EpubRendererIndex.vue | 332 ++++++++++-------- .../epub_viewer/frontend/views/TopBar.vue | 120 +++---- .../frontend/views/__tests__/TopBar.spec.js | 23 +- .../frontend/views/Html5AppRendererIndex.vue | 69 +--- .../frontend/views/PdfRendererIndex.vue | 317 ++++++++--------- .../kolibri-common/components/BaseToolbar.vue | 13 +- .../components/EmbeddedReadCard.vue | 119 +++++++ .../components/SafeHTML/index.js | 11 +- .../components/SafeHTML/style.scss | 10 +- .../components/ViewerToolbar.vue | 97 +++++ .../__tests__/ViewerToolbar.spec.js | 62 ++++ 12 files changed, 747 insertions(+), 510 deletions(-) create mode 100644 packages/kolibri-common/components/EmbeddedReadCard.vue create mode 100644 packages/kolibri-common/components/ViewerToolbar.vue create mode 100644 packages/kolibri-common/components/__tests__/ViewerToolbar.spec.js diff --git a/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue b/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue index 2cae0364fd8..ac218fd1d9e 100644 --- a/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue +++ b/kolibri/plugins/bloompub_viewer/frontend/views/BloomPubRendererIndex.vue @@ -6,28 +6,11 @@ :style="{ width: iframeWidth }" @changeFullscreen="isInFullscreen = $event" > -
- - - - {{ fullscreenText }} - -
+
@@ -181,18 +168,7 @@ diff --git a/packages/kolibri-common/components/EmbeddedReadCard.vue b/packages/kolibri-common/components/EmbeddedReadCard.vue new file mode 100644 index 00000000000..20623612bc6 --- /dev/null +++ b/packages/kolibri-common/components/EmbeddedReadCard.vue @@ -0,0 +1,119 @@ + + + + + + + diff --git a/packages/kolibri-common/components/SafeHTML/index.js b/packages/kolibri-common/components/SafeHTML/index.js index f274822a1af..199456c1030 100644 --- a/packages/kolibri-common/components/SafeHTML/index.js +++ b/packages/kolibri-common/components/SafeHTML/index.js @@ -91,15 +91,24 @@ export function createSafeHTML(customComponents = {}, { allowedOrigins } = {}) { if (component === 'ContentViewer') { childProps.embedded = true; } + // Extract class from attrs so Vue merges it with the component's + // template class instead of overriding it. In Vue 2, class + // inside attrs overrides a component's root element class. + const { class: className, ...componentAttrs } = attrs; const childVNode = h( component, { + class: className, props: childProps, - attrs, + attrs: componentAttrs, on: context.listeners, }, mapChildren(node.childNodes), ); + // Wrap embedded ContentViewers in a layout container + if (component === 'ContentViewer') { + return h('div', { class: 'embedded-content-viewer' }, [childVNode]); + } return childVNode; } diff --git a/packages/kolibri-common/components/SafeHTML/style.scss b/packages/kolibri-common/components/SafeHTML/style.scss index 5c0b04ba676..d21729a795a 100644 --- a/packages/kolibri-common/components/SafeHTML/style.scss +++ b/packages/kolibri-common/components/SafeHTML/style.scss @@ -88,9 +88,7 @@ b.safe-html { } .table-container { - width: calc(100% + 32px); - padding: 0 16px; - margin-left: -16px; + width: 100%; overflow-x: auto; } @@ -172,6 +170,12 @@ img.safe-html { opacity: 1; } +.embedded-content-viewer { + max-width: 960px; + margin: 40px auto; + overflow: hidden; +} + math.safe-html { @include text-style(normal, 32px, 130%); diff --git a/packages/kolibri-common/components/ViewerToolbar.vue b/packages/kolibri-common/components/ViewerToolbar.vue new file mode 100644 index 00000000000..e079d567a88 --- /dev/null +++ b/packages/kolibri-common/components/ViewerToolbar.vue @@ -0,0 +1,97 @@ + + + + + + + diff --git a/packages/kolibri-common/components/__tests__/ViewerToolbar.spec.js b/packages/kolibri-common/components/__tests__/ViewerToolbar.spec.js new file mode 100644 index 00000000000..4b9a4ed8f5e --- /dev/null +++ b/packages/kolibri-common/components/__tests__/ViewerToolbar.spec.js @@ -0,0 +1,62 @@ +import { render, screen } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; +import { createTranslator } from 'kolibri/utils/i18n'; +import ViewerToolbar from '../ViewerToolbar'; + +const { enterFullscreen$, exitFullscreen$ } = createTranslator( + ViewerToolbar.name, + ViewerToolbar.$trs, +); + +function renderToolbar(props = {}, slots = {}) { + return render(ViewerToolbar, { + props: { + isInFullscreen: false, + ...props, + }, + slots, + }); +} + +describe('ViewerToolbar', () => { + it('should mount', () => { + const { container } = renderToolbar(); + expect(container.firstChild).toBeTruthy(); + }); + + it('shows enter fullscreen button when not in fullscreen', () => { + renderToolbar({ isInFullscreen: false }); + expect(screen.getByText(enterFullscreen$())).toBeInTheDocument(); + }); + + it('shows exit fullscreen button when in fullscreen', () => { + renderToolbar({ isInFullscreen: true }); + expect(screen.getByText(exitFullscreen$())).toBeInTheDocument(); + }); + + it('shows primary fullscreen button when embedded', () => { + renderToolbar({ embedded: true }); + expect(screen.getByText(enterFullscreen$())).toBeInTheDocument(); + }); + + it('emits toggleFullscreen when fullscreen button is clicked', async () => { + const { emitted } = renderToolbar(); + await userEvent.click(screen.getByText(enterFullscreen$())); + expect(emitted().toggleFullscreen).toBeTruthy(); + }); + + it('renders left slot content', () => { + renderToolbar({}, { left: 'Left' }); + expect(screen.getByTestId('left-content')).toBeInTheDocument(); + }); + + it('renders center slot content', () => { + renderToolbar({}, { center: 'Center' }); + expect(screen.getByTestId('center-content')).toBeInTheDocument(); + }); + + it('renders right slot content', () => { + renderToolbar({}, { right: 'Right' }); + expect(screen.getByTestId('right-content')).toBeInTheDocument(); + }); +}); From 7b138aa64999c091627aa9f6db0c15feead4ebaf Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Sun, 22 Mar 2026 15:57:34 -0700 Subject: [PATCH 5/5] Simplify SafeHtmlImage and SafeHtmlTable attribute passing Replace v-bind="$attrs" with explicit class="safe-html" and remove inheritAttrs: false, reducing the component API surface area. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/kolibri-common/components/SafeHTML/SafeHtmlImage.vue | 3 +-- packages/kolibri-common/components/SafeHTML/SafeHtmlTable.vue | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/kolibri-common/components/SafeHTML/SafeHtmlImage.vue b/packages/kolibri-common/components/SafeHTML/SafeHtmlImage.vue index e12cbe5592f..8c1286d33da 100644 --- a/packages/kolibri-common/components/SafeHTML/SafeHtmlImage.vue +++ b/packages/kolibri-common/components/SafeHTML/SafeHtmlImage.vue @@ -6,10 +6,10 @@ >
@@ -19,8 +19,6 @@ export default { name: 'SafeHtmlTable', - inheritAttrs: false, - props: { node: { required: true,