Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Commit

Permalink
Share audio loaded state through pinia (#1529)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
sarayourfriend authored Jun 26, 2022
1 parent 515693f commit 96b061f
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 15 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 8 additions & 7 deletions src/components/VAudioTrack/VAudioTrack.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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({
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions src/components/VAudioTrack/VGlobalAudioTrack.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -85,7 +86,7 @@ export default defineComponent({
})
const setPlaying = () => {
if (hasLoaded) {
if (props.audio.hasLoaded) {
status.value = 'playing'
} else {
status.value = 'loading'
Expand Down Expand Up @@ -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 = () => {
Expand Down
6 changes: 3 additions & 3 deletions src/locales/po-files/openverse.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 ""
Expand Down
3 changes: 3 additions & 0 deletions src/models/media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends SupportedMediaType> =
Expand Down
16 changes: 16 additions & 0 deletions src/stores/media/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -391,5 +392,20 @@ export const useMediaStore = defineStore('media', {
throw new Error(errorMessage)
}
},

setMediaProperties(
type: SupportedMediaType,
id: string,
properties: Partial<DetailFromMediaType<typeof type>>
) {
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.`
)
}
},
},
})
22 changes: 22 additions & 0 deletions test/unit/specs/stores/media-store.spec.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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,
})
})
})
})
})

0 comments on commit 96b061f

Please sign in to comment.