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

#1299 Update FileList component according to DS #1311

Merged
merged 3 commits into from
Jul 3, 2024

Conversation

katarinazaprazna
Copy link
Contributor

@katarinazaprazna katarinazaprazna commented Jun 28, 2024

  • Add border to the list that displays files in rows
  • Replace div markup with an unordered list and list items
    markuphero-bsJFEzuahLKZgmNafHNj

@katarinazaprazna katarinazaprazna self-assigned this Jun 28, 2024
@katarinazaprazna katarinazaprazna linked an issue Jun 28, 2024 that may be closed by this pull request
@katarinazaprazna katarinazaprazna marked this pull request as ready for review June 28, 2024 08:23
{fileSection?.files.map((file, fileIndex) => (
// eslint-disable-next-line react/no-array-index-key
<div key={fileIndex} className="w-full">
<li key={fileIndex} className="w-full">
<FileCardWrapper fileItem={file} variant={variantFileList} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we can show the bottom divider of FIleCard based on whether it is the last child of the list or not.

Unfortunately we cannot simply use the class divide because the divider doesn't span the full width of the FileCard, so a prop needs to be drilled down through the FileCardWrapper into FileCard.

@github-actions github-actions bot added pr: needs work 🛠️ Changes requested before another review and removed pr: needs review 🙏 labels Jul 1, 2024
Copy link
Contributor

@Ty-ci Ty-ci left a comment

Choose a reason for hiding this comment

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

nice work 👍

<FileCardWrapper
fileItem={file}
variant={variantFileList}
hideBottomDivider={fileIndex === (fileSection?.files?.length ?? false) - 1}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
hideBottomDivider={fileIndex === (fileSection?.files?.length ?? false) - 1}
hideBottomDivider={fileIndex === fileSection.files.length - 1}

Operators ?. and ?? are not needed, because the files array is defined. We use these only when necessary :)

@github-actions github-actions bot added pr: fix & ship 🚢 No additional review needed before merge - some work may be required, if specified in last review and removed pr: needs review 🙏 pr: needs work 🛠️ Changes requested before another review labels Jul 2, 2024
Copy link

github-actions bot commented Jul 3, 2024

Test build pipeline info 🚀

Changes in the code and tag info:

➡️ Changes in strapi: false

➡️ Changes in next: true

We are going to build 🚢

🔜 next

🥳 Bratiska-cli successfully built an image from path: next/

@katarinazaprazna katarinazaprazna merged commit c9e1b98 into master Jul 3, 2024
8 checks passed
@katarinazaprazna katarinazaprazna deleted the 1299/update-filelist branch July 3, 2024 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix & ship 🚢 No additional review needed before merge - some work may be required, if specified in last review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update FileList according to DS
2 participants