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

16 projects description #40

Merged
merged 15 commits into from
Jul 8, 2024
Merged

16 projects description #40

merged 15 commits into from
Jul 8, 2024

Conversation

nroh555
Copy link
Contributor

@nroh555 nroh555 commented Jun 26, 2024

Context

Created the projects description page that lists all the projects and their corresponding information from the CMS. Below is a screenshot of what it looks like

![image](https://github.com/UoaWDCC/wdcc-website-v3/assets/100507962/6df793d6-c488-402b-b0dc-54d97abbfab8

How To Review

Visit http://localhost:4321/projects once you have populate the CMS with some data

@nroh555 nroh555 requested a review from chrisv09 June 26, 2024 08:00
Copy link
Contributor

@Oculux314 Oculux314 left a comment

Choose a reason for hiding this comment

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

Ooo this looks cool! Just be aware of some bugs:

  • Site breaks when there are no projects
    image
  • If a Strapi user doesn't upload an image in exactly a small format this error triggers:
    image
  • Consider truncating text when the title or description overflows
    image
  • Consider adding the description to the CMS so the 2026 team doesn't have to come read all our messy code 😁
  • I recommend modifying global.css not editing header styles raw in the HTML since these styles are reused across the project
    image
  • Remember to use the tailwind.config custom colors!
    image
  • And don't forget to yarn lint!

Love the modularisation, thought for extra requirements later on (e.g. multiple demos) and responsiveness of the page!

Okay sorry for the spam ik it's a lot of nits but it'll make the website better and more resistant to possible issues from a team that hasn't worked on this project! P.s. did you still want me to help style the project card?

Copy link
Contributor

@chrisv09 chrisv09 left a comment

Choose a reason for hiding this comment

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

Looks good

I added the descriptions of the projects page as a single type on Strapi
Copy link
Contributor

@TrissyG TrissyG left a comment

Choose a reason for hiding this comment

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

Only 2 gripes are that there seems to be lack of styling configurability for Projects-Description using the (or any default styles applied when the .md is parsed into corresponding HTML tags?)
image
Responsiveness on the card is mostly good but the technologies hide behind the button:
image

Rest of the PR contents LGTM so happy to merge and hotfix (open a bug issue) due to dependencies

Copy link
Contributor

@Oculux314 Oculux314 left a comment

Choose a reason for hiding this comment

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

Thanks Chris! It looks fabulous 💚

I added a commit which is just a lint btw

@chrisv09 chrisv09 merged commit cb75a90 into main Jul 8, 2024
1 check passed
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.

4 participants