Skip to content

Commit 6b87911

Browse files
authored
Simplify media FetchState and fix fetching errors (#5323)
* Refactor FetchState to use discriminated union Fixes #5322 --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/WordPress/openverse/issues/5322?shareId=XXXX-XXXX-XXXX-XXXX). * Fix loading labels, results, no result on later pages * Make `canLoadMore` external prop * Fix collection header stories * Refactor i18n search result count labels * Improve asyncData error in search.vue
1 parent 56be647 commit 6b87911

31 files changed

+578
-565
lines changed

frontend/nuxt.config.ts

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export default defineNuxtConfig({
1212
port: 8443,
1313
host: "0.0.0.0",
1414
},
15+
devtools: { enabled: true },
1516
imports: {
1617
autoImport: false,
1718
},

frontend/shared/types/fetch-state.ts

+3-6
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ export interface FetchingError {
2727
details?: Record<string, string>
2828
}
2929

30-
export interface FetchState {
31-
isFetching: boolean
32-
hasStarted?: boolean
33-
isFinished?: boolean
34-
fetchingError: FetchingError | null
35-
}
30+
export type FetchState =
31+
| { status: "idle" | "fetching" | "success"; error: null }
32+
| { status: "error"; error: FetchingError }

frontend/src/components/VCollectionHeader/VCollectionHeader.vue

+3-7
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,12 @@ const showCollectionExternalLink = computed(() => {
8989
return Boolean(props.collectionParams.collection !== "tag" && url.value)
9090
})
9191
92-
const { getI18nCollectionResultCountLabel } = useI18nResultsCount()
92+
const showLoading = computed(() => mediaStore.showLoading)
93+
const { getI18nCollectionResultCountLabel } = useI18nResultsCount(showLoading)
9394
9495
const resultsLabel = computed(() => {
95-
if (mediaStore.resultCount === 0 && mediaStore.fetchState.isFetching) {
96-
return ""
97-
}
98-
const resultsCount = mediaStore.results[props.mediaType].count
99-
10096
return getI18nCollectionResultCountLabel(
101-
resultsCount,
97+
mediaStore.results[props.mediaType].count,
10298
props.mediaType,
10399
props.collectionParams.collection
104100
)

frontend/src/components/VCollectionHeader/meta/VCollectionHeader.stories.ts

+4
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ export const AllCollections: Omit<Story, "args"> = {
9595
const mediaStore = useMediaStore()
9696
mediaStore.$patch({
9797
results: { image: { count: 240 } },
98+
mediaFetchState: {
99+
image: { status: "success", error: null },
100+
audio: { status: "success", error: null },
101+
},
98102
})
99103
return () =>
100104
h(
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,29 @@
11
<script setup lang="ts">
22
import { useNuxtApp } from "#imports"
3-
import { computed } from "vue"
43
54
import type { SupportedMediaType } from "#shared/constants/media"
65
import useSearchType from "~/composables/use-search-type"
76
import { useHydrating } from "~/composables/use-hydrating"
8-
import { useI18nResultsCount } from "~/composables/use-i18n-utilities"
97
108
import VButton from "~/components/VButton.vue"
119
import VIcon from "~/components/VIcon/VIcon.vue"
1210
1311
const props = defineProps<{
1412
mediaType: SupportedMediaType
15-
/**
16-
* Current search term for aria-label.
17-
*/
18-
searchTerm: string
19-
/**
20-
* The number of results that the search returned. The link
21-
* will be disabled if this value is zero.
22-
*/
23-
resultsCount: number
2413
/**
2514
* The route target of the link.
2615
*/
27-
to?: string
16+
to: string | undefined
17+
labels: {
18+
aria: string
19+
visible: string
20+
}
2821
}>()
2922
3023
defineEmits<{
3124
"shift-tab": [KeyboardEvent]
3225
}>()
3326
34-
const { getI18nCount, getI18nContentLinkLabel } = useI18nResultsCount()
35-
const resultsCountLabel = computed(() => getI18nCount(props.resultsCount))
36-
37-
const resultsAriaLabel = computed(() =>
38-
getI18nContentLinkLabel(props.resultsCount, props.searchTerm, props.mediaType)
39-
)
40-
4127
const { activeType } = useSearchType({ component: "VContentLink" })
4228
const { $sendCustomEvent } = useNuxtApp()
4329
@@ -56,7 +42,7 @@ const { doneHydrating } = useHydrating()
5642
<VButton
5743
as="VLink"
5844
:href="to"
59-
:aria-label="resultsAriaLabel"
45+
:aria-label="labels.aria"
6046
variant="bordered-gray"
6147
size="disabled"
6248
:disabled="!doneHydrating"
@@ -70,7 +56,7 @@ const { doneHydrating } = useHydrating()
7056
</p>
7157
<span
7258
class="label-regular sm:description-regular text-secondary group-hover/button:text-default sm:ms-auto"
73-
>{{ resultsCountLabel }}</span
59+
>{{ labels.visible }}</span
7460
>
7561
</VButton>
7662
</template>

frontend/src/components/VHeader/VHeaderDesktop.vue

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const uiStore = useUiStore()
2929
3030
const isSidebarVisible = inject<Ref<boolean>>(IsSidebarVisibleKey)
3131
32-
const isFetching = computed(() => mediaStore.fetchState.isFetching)
32+
const isFetching = computed(() => mediaStore.isFetching)
3333
3434
const { $sendCustomEvent } = useNuxtApp()
3535

frontend/src/components/VHeader/VHeaderMobile/VHeaderMobile.vue

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ const isInputFocused = ref(false)
6363
const contentSettingsOpen = ref(false)
6464
6565
const appliedFilterCount = computed(() => searchStore.appliedFilterCount)
66-
const isFetching = computed(() => mediaStore.fetchState.isFetching)
66+
const isFetching = computed(() => mediaStore.isFetching)
6767
6868
/**
6969
* The selection range of the input field. Used to make sure that the cursor

frontend/src/components/VLoadMore.vue

+4-14
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ import { useSearchStore } from "~/stores/search"
1212
1313
import VButton from "~/components/VButton.vue"
1414
15-
const props = defineProps<
15+
defineProps<
1616
SingleResultProps & {
1717
searchType: SupportedSearchType
18-
isFetching: boolean
18+
canLoadMore: boolean
1919
}
2020
>()
2121
@@ -39,13 +39,7 @@ const eventPayload = computed(() => {
3939
}
4040
})
4141
42-
/**
43-
* Whether we should show the "Load more" button.
44-
* If the fetching for the current query has started, there is at least
45-
* 1 page of results, there has been no fetching error, and there are
46-
* more results to fetch, we show the button.
47-
*/
48-
const canLoadMore = computed(() => mediaStore.canLoadMore)
42+
const isFetching = computed(() => mediaStore.isFetching)
4943
5044
const reachResultEndEventSent = ref(false)
5145
/**
@@ -56,10 +50,6 @@ const reachResultEndEventSent = ref(false)
5650
*
5751
*/
5852
const onLoadMore = async () => {
59-
if (props.isFetching) {
60-
return
61-
}
62-
6353
reachResultEndEventSent.value = false
6454
6555
$sendCustomEvent("LOAD_MORE_RESULTS", eventPayload.value)
@@ -76,7 +66,7 @@ const sendReachResultEnd = () => {
7666
}
7767
7868
const buttonLabel = computed(() =>
79-
props.isFetching ? t("browsePage.loading") : t("browsePage.load")
69+
isFetching.value ? t("browsePage.loading") : t("browsePage.load")
8070
)
8171
const mainPageElement = ref<HTMLElement | null>(null)
8272
onMounted(() => {

frontend/src/components/VMediaInfo/VRelatedMedia.vue

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ watch(
3333
{ immediate: true }
3434
)
3535
36-
const isFetching = computed(() => relatedMediaStore.fetchState.isFetching)
36+
const isFetching = computed(() => relatedMediaStore.isFetching)
3737
const showRelated = computed(
3838
() => results.value.items.length > 0 || isFetching.value
3939
)

frontend/src/components/VSearchResultsGrid/VCollectionResults.vue

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ defineProps<{
1414
isFetching: boolean
1515
searchTerm: string
1616
creatorUrl?: string
17+
canLoadMore: boolean
1718
}>()
1819
1920
defineEmits<{ "load-more": [] }>()
@@ -40,6 +41,7 @@ defineEmits<{ "load-more": [] }>()
4041
<template #footer>
4142
<footer class="mb-6 mt-4 lg:mb-10">
4243
<VLoadMore
44+
:can-load-more="canLoadMore"
4345
:search-type="results.type"
4446
kind="collection"
4547
:search-term="searchTerm"

frontend/src/components/VSearchResultsGrid/VSearchResults.vue

+15-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script setup lang="ts">
2-
import { useI18n } from "#imports"
2+
import { useI18n, useI18nResultsCount } from "#imports"
33
import { computed } from "vue"
44
55
import type { SupportedMediaType } from "#shared/constants/media"
@@ -33,7 +33,19 @@ const collectionLabel = computed(() => {
3333
const contentLinkPath = (mediaType: SupportedMediaType) =>
3434
searchStore.getSearchPath({ type: mediaType })
3535
36+
const showLoading = computed(() => mediaStore.showLoading)
37+
38+
const { getResultCountLabels } = useI18nResultsCount(showLoading)
39+
40+
const labels = (
41+
count: number,
42+
searchTerm: string,
43+
mediaType: SupportedMediaType
44+
) => getResultCountLabels(count, mediaType, searchTerm)
45+
3646
const resultCounts = computed(() => mediaStore.resultCountsPerMediaType)
47+
48+
const canLoadMore = computed(() => mediaStore.canLoadMore)
3749
</script>
3850

3951
<template>
@@ -62,16 +74,16 @@ const resultCounts = computed(() => mediaStore.resultCountsPerMediaType)
6274
<VContentLink
6375
v-for="[mediaType, count] in resultCounts"
6476
:key="mediaType"
77+
:labels="labels(count, searchTerm, mediaType)"
6578
:media-type="mediaType"
66-
:search-term="searchTerm"
67-
:results-count="count"
6879
:to="contentLinkPath(mediaType)"
6980
/>
7081
</div>
7182
</template>
7283
<template #footer>
7384
<footer class="mb-6 mt-4 lg:mb-10">
7485
<VLoadMore
86+
:can-load-more="canLoadMore"
7587
:search-type="results.type"
7688
kind="search"
7789
:search-term="searchTerm"

frontend/src/components/meta/CustomButtonComponents.stories.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import { h } from "vue"
22

3-
import { useMediaStore } from "~/stores/media"
4-
53
import VLoadMore from "~/components/VLoadMore.vue"
64

75
import type { StoryObj } from "@storybook/vue3"
@@ -10,16 +8,14 @@ const Template: Story = {
108
render: (args) => ({
119
components: { VLoadMore },
1210
setup() {
13-
const mediaStore = useMediaStore()
14-
mediaStore._startFetching("image")
15-
mediaStore.results.image.count = 1
1611
return () =>
1712
h("div", { class: "flex p-4", id: "wrapper" }, [
1813
h(VLoadMore, {
1914
kind: "search",
2015
searchType: "image",
2116
searchTerm: "cat",
2217
isFetching: false,
18+
canLoadMore: true,
2319
...args,
2420
}),
2521
])

frontend/src/composables/use-collection.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export const useCollection = <T extends SupportedMediaType>({
1818
const searchStore = useSearchStore()
1919

2020
const collectionParams = computed(() => searchStore.collectionParams)
21-
const isFetching = computed(() => mediaStore.fetchState.isFetching)
21+
const isFetching = computed(() => mediaStore.isFetching)
2222

2323
const media = ref(mediaStore.resultItems[mediaType]) as Ref<ResultType[]>
2424
const creatorUrl = ref<string>()
@@ -38,9 +38,8 @@ export const useCollection = <T extends SupportedMediaType>({
3838
shouldPersistMedia: false,
3939
}
4040
) => {
41-
media.value = (await mediaStore.fetchMedia({
42-
shouldPersistMedia,
43-
})) as ResultType[]
41+
const results = await mediaStore.fetchMedia({ shouldPersistMedia })
42+
media.value = results.items as ResultType[]
4443
creatorUrl.value =
4544
media.value.length > 0 ? media.value[0].creator_url : undefined
4645
return media.value
@@ -53,6 +52,7 @@ export const useCollection = <T extends SupportedMediaType>({
5352
await fetchMedia()
5453
})
5554

55+
const canLoadMore = computed(() => mediaStore.canLoadMore)
5656
const loadMore = async () => {
5757
await fetchMedia({ shouldPersistMedia: true })
5858
}
@@ -71,5 +71,6 @@ export const useCollection = <T extends SupportedMediaType>({
7171
media,
7272
fetchMedia,
7373
loadMore,
74+
canLoadMore,
7475
}
7576
}

0 commit comments

Comments
 (0)