Add product gallery + basic CSS styling #246
Conversation
using-pagination/src/pages/index.js
Outdated
| function SingleProduct({product, index}) { | ||
| return ( | ||
| <div style={singleProductStyle}> | ||
| <img src={`https://via.placeholder.com/300x200?text=${product.name}`} width={300} height={200} alt={product.name} title={product.name} /> |
There was a problem hiding this comment.
Why are we using placeholder? The project has images with each product.
There was a problem hiding this comment.
The original code didn't have images in it, so I didn't know. 😅 Could you please invite me to the project?
| import { gql } from 'graphql-request'; | ||
|
|
||
| import { graphcms } from '../../lib/_graphcms'; | ||
|
|
||
| const limit = 1; | ||
|
|
||
| const singleProductStyle = { |
There was a problem hiding this comment.
We want to improve styling for all our examples. I'm happy if you want to add these here, but FYI, these may get removed with an updated design when we get some design resources to help here.
There was a problem hiding this comment.
That's definitely a thing! Do you already have basic styling from other example that I could use here?
There was a problem hiding this comment.
these may get removed with an updated design when we get some design resources to help here
| function SingleProduct({product}) { | ||
| return ( | ||
| <div style={singleProductStyle} key={product.id}> | ||
| <img src={`https://via.placeholder.com/300x200?text=${product.name}`} width={300} height={200} alt={product.name} title={product.name} /> |
There was a problem hiding this comment.
Again. Products have images. You can query these 😃
There was a problem hiding this comment.
Looks good.
I made a comment about the image of the products.
Since we have the product images already, you could query for those.
However, where do you stop with that? These examples were intended to be isolated, and to the point. In this case "using pagination". We already have examples on using Next.js etc that shows how to query/display images.
Personally adding images here is overkill. If we're seeing more people ask for this example then it would make sense, but I don't see a huge demand of people asking for a combined example that they can't put together themselves using the isolated examples.
| return ( | ||
| <div style={singleProductStyle} key={product.id}> | ||
| <img src={`https://via.placeholder.com/300x200?text=${product.name}`} width={300} height={200} alt={product.name} title={product.name} /> | ||
| <img src={image.url} width={200} height={200} alt={product.name} title={product.name} /> |
There was a problem hiding this comment.
It's not recommended you use <img /> if you know the width/height but instead use Next.js Image.
However, I would not show an image at all here. We have an example for that.
There was a problem hiding this comment.
@notrab we're using Next.js 9 on this project and next/image was introduced in v10. Unless we upgrade it, it's not possible.
There was a problem hiding this comment.
That's not really relevant though. We should aim to keep these updated regardless.
Small improvements based on user feedback