The Wayback Machine - https://web.archive.org/web/20210815082124/https://github.com/UnlyEd/next-right-now/issues/196
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

Migrate GraphCMSAsset component to use Next.js 10 Image component #196

Open
Vadorequest opened this issue Nov 1, 2020 · 5 comments
Open

Migrate GraphCMSAsset component to use Next.js 10 Image component #196

Vadorequest opened this issue Nov 1, 2020 · 5 comments

Comments

@Vadorequest
Copy link
Member

@Vadorequest Vadorequest commented Nov 1, 2020

Goal

Use the newer next/Image component instead of a simple img tag within the GraphCMSAsset component.

The goal is to leverage the new capabilities from the Image component, such as image optimization.

Affects v2-mst-aptd-gcms-lcz-sty preset only.
Similar to #195

Status

On-hold until the below issues are fixed.

Issues

Awaiting real use-cases. Lack real-use feedback.

The new next/Image component relies on new properties that need to update the CMS data schema.

  • width
  • height
  • sizes? Might not be useful in the CMS but rather in the app source code directly?
  • loading? Might not be useful in the CMS but rather in the app source code directly?
  • priority? Might not be useful in the CMS but rather in the app source code directly?
    ... more from https://nextjs.org/docs/api-reference/next/image

I'm concerned about the usage of the GraphCMSAsset component. I'm wondering if it isn't too complicated.

The main goal of this component is to display an image that's defined on the CMS (GraphCMS here) and to wrap it into a link to make the image clickable. Then, because the asset might not be defined on the CMS, the component had to handle default values. Then, to fit more use-cases, it had to handle overrides of asset and link.

Now, the CMS must be updated to add the required width/height properties, and other properties like sizes, loading, etc. It's becoming much more complicated and I lack feedback experience for real use-case.

The goal of this component has grown and became unclear. I wonder if it should be updated, removed or left as-it.

References:

@Vadorequest
Copy link
Member Author

@Vadorequest Vadorequest commented Dec 20, 2020

I won't have time to work on this until 1-3 months. But PR are welcome!

I may even not work on it at all, considering I don't use this preset anymore and can't afford to spend the time optimizing something I don't use.

@samuelcastro
Copy link
Contributor

@samuelcastro samuelcastro commented Mar 2, 2021

Hey @Vadorequest are you still planning to work on this? It'd be super helpful to have it.

@Vadorequest
Copy link
Member Author

@Vadorequest Vadorequest commented Mar 2, 2021

@Vadorequest
Copy link
Member Author

@Vadorequest Vadorequest commented Mar 3, 2021

For the record, the official Nextjs + GraphCMS example has been updated and might be a good starting point.

vercel/next.js#20772

@samuelcastro
Copy link
Contributor

@samuelcastro samuelcastro commented Mar 3, 2021

This is definitely a good start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment