From 96b061f411fbc73ae55980e2b6f9ecd358931762 Mon Sep 17 00:00:00 2001 From: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com> Date: Mon, 27 Jun 2022 09:49:58 +1000 Subject: [PATCH] Share audio loaded state through pinia (#1529) * Share audio loaded state through pinia * Remove unnecessary status confirmations * Move loaded state into the audio object itself * Fix scripts passing flags to other scripts * Update POT * Get media items safely --- package.json | 4 ++-- src/components/VAudioTrack/VAudioTrack.vue | 15 +++++++------ .../VAudioTrack/VGlobalAudioTrack.vue | 9 +++++--- src/locales/po-files/openverse.pot | 6 ++--- src/models/media.ts | 3 +++ src/stores/media/index.ts | 16 ++++++++++++++ test/unit/specs/stores/media-store.spec.js | 22 +++++++++++++++++++ 7 files changed, 60 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index d7bf49189d..10382cee2f 100644 --- a/package.json +++ b/package.json @@ -27,14 +27,14 @@ "prod": "pnpm build:only && pnpm start", "storybook": "nuxt storybook --port=54000", "storybook:build": "nuxt storybook build", - "storybook:smoke": "pnpm storybook -- --ci --smoke-test", + "storybook:smoke": "pnpm storybook --ci --smoke-test", "tcv": "tailwind-config-viewer --port 54001", "tcv:build": "tailwind-config-viewer export ./.tcv-export", "talkback": "node ./test/proxy.js", "pretest": "pnpm install", "test": "pnpm test:unit && pnpm test:playwright", "test:unit": "jest", - "test:unit:watch": "pnpm test:unit -- --collectCoverage=false --watch", + "test:unit:watch": "pnpm test:unit --collectCoverage=false --watch", "test:playwright": "./bin/playwright.sh", "test:playwright:local": "playwright test -c test/playwright", "test:playwright:debug": "PWDEBUG=1 pnpm test:playwright:local", diff --git a/src/components/VAudioTrack/VAudioTrack.vue b/src/components/VAudioTrack/VAudioTrack.vue index a48ea7bc76..ca10c7d606 100644 --- a/src/components/VAudioTrack/VAudioTrack.vue +++ b/src/components/VAudioTrack/VAudioTrack.vue @@ -178,7 +178,7 @@ export default defineComponent({ * We can only create the local audio object on the client, * so the initialization of this variable is hidden inside * the `initLocalAudio` function which is only called when - * playback is first requested or when the track if first seeked. + * playback is first requested or when the track is first seeked. * * However, when navigating to an audio result page, if * the globally active audio already matches the result @@ -201,19 +201,21 @@ export default defineComponent({ } } - let hasLoaded = false + const mediaStore = useMediaStore() const setLoaded = () => { - hasLoaded = true + mediaStore.setMediaProperties('audio', props.audio.id, { + hasLoaded: true, + }) status.value = 'playing' } const setWaiting = () => { status.value = 'loading' } const setPlaying = () => { - if (!hasLoaded) { - status.value = 'loading' - } else { + if (props.audio.hasLoaded) { status.value = 'playing' + } else { + status.value = 'loading' } activeAudio.obj.value = localAudio activeMediaStore.setActiveMediaItem({ @@ -279,7 +281,6 @@ export default defineComponent({ localAudio?.removeEventListener(name, fn) ) - const mediaStore = useMediaStore() if ( route.value.params.id === props.audio.id || mediaStore.getItemById(AUDIO, props.audio.id) diff --git a/src/components/VAudioTrack/VGlobalAudioTrack.vue b/src/components/VAudioTrack/VGlobalAudioTrack.vue index 583bfad77c..6e666bfdce 100644 --- a/src/components/VAudioTrack/VGlobalAudioTrack.vue +++ b/src/components/VAudioTrack/VGlobalAudioTrack.vue @@ -39,6 +39,7 @@ import { defaultRef } from '~/composables/default-ref' import { useI18n } from '~/composables/use-i18n' import { useActiveMediaStore } from '~/stores/active-media' +import { useMediaStore } from '~/stores/media' import type { AudioDetail } from '~/models/media' import type { AudioStatus } from '~/constants/audio' @@ -85,7 +86,7 @@ export default defineComponent({ }) const setPlaying = () => { - if (hasLoaded) { + if (props.audio.hasLoaded) { status.value = 'playing' } else { status.value = 'loading' @@ -118,9 +119,11 @@ export default defineComponent({ } } - let hasLoaded = false + const mediaStore = useMediaStore() const setLoaded = () => { - hasLoaded = true + mediaStore.setMediaProperties('audio', props.audio.id, { + hasLoaded: true, + }) status.value = 'playing' } const setWaiting = () => { diff --git a/src/locales/po-files/openverse.pot b/src/locales/po-files/openverse.pot index 82134d9681..4d6d44a760 100644 --- a/src/locales/po-files/openverse.pot +++ b/src/locales/po-files/openverse.pot @@ -4,7 +4,7 @@ msgid "" msgstr "" "Project-Id-Version: Openverse \n" "Report-Msgid-Bugs-To: https://github.com/wordpress/openverse/issues \n" -"POT-Creation-Date: 2022-06-18T04:57:35+00:00\n" +"POT-Creation-Date: 2022-06-22T23:37:38+00:00\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" @@ -258,13 +258,13 @@ msgid "Loading" msgstr "" #. Do not translate words between ### ###. -#: src/components/VAudioTrack/VAudioTrack.vue:416 +#: src/components/VAudioTrack/VAudioTrack.vue:417 msgctxt "audio-track.aria-label" msgid "###title### - Audio Player" msgstr "" #. Do not translate words between ### ###. -#: src/components/VAudioTrack/VAudioTrack.vue:413 +#: src/components/VAudioTrack/VAudioTrack.vue:414 msgctxt "audio-track.aria-label-interactive" msgid "###title### - audio player - press the spacebar to play and pause a preview of the audio" msgstr "" diff --git a/src/models/media.ts b/src/models/media.ts index f2014281f3..140d2ded1f 100644 --- a/src/models/media.ts +++ b/src/models/media.ts @@ -63,6 +63,9 @@ export interface AudioDetail extends Media { alt_files?: { provider: string; filetype: string }[] peaks?: number[] waveform?: string + + // Set and managed by the frontend client-side + hasLoaded?: boolean } export type DetailFromMediaType = diff --git a/src/stores/media/index.ts b/src/stores/media/index.ts index ad8d8d2881..44a8b743ab 100644 --- a/src/stores/media/index.ts +++ b/src/stores/media/index.ts @@ -2,6 +2,7 @@ import { defineStore } from 'pinia' import axios from 'axios' +import { warn } from '~/utils/console' import { hash, rand as prng } from '~/utils/prng' import prepareSearchQueryParams from '~/utils/prepare-search-query-params' import type { DetailFromMediaType, Media } from '~/models/media' @@ -391,5 +392,20 @@ export const useMediaStore = defineStore('media', { throw new Error(errorMessage) } }, + + setMediaProperties( + type: SupportedMediaType, + id: string, + properties: Partial> + ) { + const item = this.getItemById(type, id) + if (item) { + Object.assign(item, properties) + } else { + warn( + `Attempted to update media item ${type} ${id} but could not find it.` + ) + } + }, }, }) diff --git a/test/unit/specs/stores/media-store.spec.js b/test/unit/specs/stores/media-store.spec.js index 909cd9a8ca..30c71717eb 100644 --- a/test/unit/specs/stores/media-store.spec.js +++ b/test/unit/specs/stores/media-store.spec.js @@ -1,5 +1,7 @@ import { createPinia, setActivePinia } from 'pinia' +import { deepClone } from '~/utils/clone' + import { initialResults, useMediaStore } from '~/stores/media' import { useSearchStore } from '~/stores/search' import { ALL_MEDIA, AUDIO, IMAGE, supportedMediaTypes } from '~/constants/media' @@ -393,5 +395,25 @@ describe('Media Store', () => { mediaStore.handleMediaError({ mediaType: AUDIO, error }) ).rejects.toThrow(error.message) }) + + describe('setMediaProperties', () => { + it('merges the existing media item together with the properties passed in allowing overwriting', () => { + const mediaStore = useMediaStore() + mediaStore.results.audio = testResult(AUDIO) + + const existingMediaItem = deepClone( + mediaStore.getItemById(AUDIO, uuids[0]) + ) + const hasLoaded = Symbol() + mediaStore.setMediaProperties(AUDIO, uuids[0], { + hasLoaded, + }) + + expect(mediaStore.getItemById(AUDIO, uuids[0])).toMatchObject({ + ...existingMediaItem, + hasLoaded, + }) + }) + }) }) })