Skip to content

posters list whithout filter#4

Draft
Cimanel wants to merge 2 commits into
masterfrom
postersList
Draft

posters list whithout filter#4
Cimanel wants to merge 2 commits into
masterfrom
postersList

Conversation

@Cimanel
Copy link
Copy Markdown
Owner

@Cimanel Cimanel commented Apr 21, 2022

  • posters list
  • filter panel
    image

@Cimanel Cimanel added the WIP label Apr 21, 2022
</Box>
);

const TitlebarImageList = () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That's a strange name for a component with no titlebar 🤔

Comment thread src/ressources/products/productList.tsx Outdated
Comment on lines +54 to +55
src={`${item.image}`}
srcSet={`${item.image}`}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
src={`${item.image}`}
srcSet={`${item.image}`}
src={item.image}
srcSet={item.image}

Also, why do you need srcSet ?

Comment thread src/ressources/products/productList.tsx Outdated
/>
<ImageListItemBar
title={item.reference}
subtitle={`${item.width}x${item.height}, ${item.price} $US`}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be nice not to have $US as a constant. Ideally a component (MUI or RA) making use of { style: "currency", currency: "USD" } for the formatting.

Comment thread src/ressources/products/productList.tsx Outdated
rowHeight={180}
>
{data.map((item) => (
<ImageListItem key={item.image}>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using item.image as a key is a bit weird. Why not use item.id?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants