Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate one media store into single media type stores and useAllMedia store. #587

Open
1 task
obulat opened this issue Mar 27, 2022 · 0 comments
Open
1 task
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend

Comments

@obulat
Copy link
Contributor

obulat commented Mar 27, 2022

This issue description is largely based on @sarayourfriend 's comment on the media store Pinia conversion.

Problem

Currently, we have a single large store to keep data about the media search results of all type. This means that for all actions, we have to pass mediaType parameter to apply changes to the correct data. The actions and getters for allMedia are quite different from the single media type actions, and they are all bundled together in a large store.
Having all our media results in a single store:

  • introduces some complications for types
  • is problematic for long term maintainability
  • increases the complexity a lot.

Description

If we changed the media store to be generic, something like this:

const createResultsStore = <T extends SupportedMediaType>(mediaType: T) => createStore(`${mediaType}-results`, () => {
  const state: ResultState<DetailFromMediaType<T>> = reactive({
    results: // unnested,
    fetchState: // unnested,
    // no need for audio or image key at the root
  })

  const getItemById = (id: string) => {
    return state.results.items[id]
  }

  const resultList = computed(() => Object.values(state.results.items))

  const resultCount = computed(() => state.results.count)

  // etc...
})

Then useImageStore and useAudioStore are separate entities. A third useAllResultsStore would aggregate the two (and any future stores) in a generic fashion.

This would remove all the [mediaType] keying we have to do now as well as the conditionals around ALL_MEDIA that we have to do now.

The search page itself would decide which store to use based on the search type, rather than having the media store opaquely decide which results to return.

Another part of the store that keeps single media details, will also need to be split into its own store.

Implementation

  • 🙋 I would be interested in implementing this feature.
@obulat obulat added 🟩 priority: low Low priority and doesn't need to be rushed 💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users labels Mar 27, 2022
@obulat obulat transferred this issue from WordPress/openverse-frontend Feb 22, 2023
@obulat obulat added the 🧱 stack: frontend Related to the Nuxt frontend label Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend
Projects
Status: 📋 Backlog
Development

No branches or pull requests

1 participant