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

feat(composable): add useMedia composable #1370

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mdanilowicz
Copy link
Collaborator

Description

Add useMedia composable

Type of change

ToDo's

Screenshots (if applicable)

Additional context

Copy link

vercel bot commented Oct 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
frontends-demo 🔄 Building (Inspect) Visit Preview Dec 3, 2024 10:49am
1 Skipped Deployment
Name Status Preview Updated (UTC)
shopware-frontends-docs ⬜️ Skipped (Inspect) Dec 3, 2024 10:49am

Copy link

pkg-pr-new bot commented Oct 18, 2024

Open in Stackblitz

@shopware/api-client

npm i https://pkg.pr.new/shopware/frontends/@shopware/api-client@1370

@shopware/api-gen

npm i https://pkg.pr.new/shopware/frontends/@shopware/api-gen@1370

@shopware-pwa/cms-base

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/cms-base@1370

@shopware-pwa/composables-next

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/composables-next@1370

@shopware-pwa/helpers-next

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/helpers-next@1370

@shopware-pwa/nuxt3-module

npm i https://pkg.pr.new/shopware/frontends/@shopware-pwa/nuxt3-module@1370

commit: c1f5790

pnpm-lock.yaml Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pnpm-lock.yaml changes needed? 🤔

Copy link
Collaborator

@mkucmus mkucmus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the composable we all needed! 🥇

I left some small notes - not crucial but who knows :)

*
* @returns {UseMedia} media composable
*/
export function useMedia(): UseMedia {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see what kind of media this composable is made for. Maybe we could add a parameter in type Media for composable function, and then we could extract ids from argument and replace/merge it with ids provided in fetchMedia function which could then become optional.

the second option to tell something more about the purpose is providing an example in functions' description and changeset file.

import { useShopwareContext } from "#imports";
import type { operations } from "#shopware";

interface UseMedia {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's our first interface within composables, maybe we could keep it consistent and change to the type 😉

@mdanilowicz mdanilowicz marked this pull request as draft October 21, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants