Skip to content

Conversation

christina-baldwin
Copy link

@christina-baldwin christina-baldwin changed the title PR for review & submission on fportfolio project PR for review & submission on portfolio project Apr 27, 2025
@christina-baldwin
Copy link
Author

Note (27/04/2025): this site has no functionality (e.g., clicking "see more" and it loading more content), its only a design as that is what I understood was the goal for week 12 + 13 and useEffect is week 14.

Copy link

@JasminHed JasminHed left a comment

Choose a reason for hiding this comment

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

Hello Christina,
Great job on your portfolio, it really feels like you and that is amazing.

  • The structure you have with folders and files are very easy to follow
  • What does the media file do with the different breakpoints? Did not get this from the live sessions but it seems you did :)
  • Code feels readable and understandable, great!

@christina-baldwin
Copy link
Author

Hello Christina, Great job on your portfolio, it really feels like you and that is amazing.

* The structure you have with folders and files are very easy to follow

* What does the media file do with the different breakpoints? Did not get this from the live sessions but it seems you did :)

* Code feels readable and understandable, great!

Hi! Thanks Jasmin!
The media file with the breakpoints was shown in one of the lectures. It basically makes it simpler to do media queries so you don't have to write the widths every time and you just import the file and use the name associated with the different breakpoints.

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

A11y
100% in Lighthouse ⭐

Content

  • Why no images for the projects? Doesn’t have to be screenshots, it can be for graphic purposes

Responsiveness

  • Requirement not met: Your portfolio should be responsive (it should look good on devices from 320px width up to at least 1600px)

Regarding the requirement: Your portfolio should follow the Figma design

  • Double check all spacings in the design using dev mode. You’ll see that most sections have a top/bottom padding of 128px on desktop for example. Align your sections (left/right) according to the design.
  • Pay attention to all details, e.g. the Skills are left aligned in desktop, the tags are wider, the buttons have more padding and another font etc.

Codewise
You have a good structure of your project and you’ve used components and props in a good way. Nice usage of useEffect and useRef ⭐

Changes requested
Almost there, but needs some additional attention:

  • Responsiveness
  • Do another take on the design

@christina-baldwin
Copy link
Author

A11y 100% in Lighthouse ⭐

Content

* Why no images for the projects? Doesn’t have to be screenshots, it can be for graphic purposes

Responsiveness

* Requirement not met: Your portfolio should be responsive (it should look good on devices from 320px width up to at least 1600px)

Regarding the requirement: Your portfolio should follow the Figma design

* Double check all spacings in the design using dev mode. You’ll see that most sections have a top/bottom padding of 128px on desktop for example. Align your sections (left/right) according to the design.

* Pay attention to all details, e.g. the Skills are left aligned in desktop, the tags are wider, the buttons have more padding and another font etc.

Codewise You have a good structure of your project and you’ve used components and props in a good way. Nice usage of useEffect and useRef ⭐

Changes requested Almost there, but needs some additional attention:

* Responsiveness

* Do another take on the design

I chose to not have project images in my portfolio, the design reflects that, do I need to have images for my projects in order to pass this project?

I will work on the comments.

Thanks!

@christina-baldwin
Copy link
Author

A11y 100% in Lighthouse ⭐

Content

* Why no images for the projects? Doesn’t have to be screenshots, it can be for graphic purposes

Responsiveness

* Requirement not met: Your portfolio should be responsive (it should look good on devices from 320px width up to at least 1600px)

Regarding the requirement: Your portfolio should follow the Figma design

* Double check all spacings in the design using dev mode. You’ll see that most sections have a top/bottom padding of 128px on desktop for example. Align your sections (left/right) according to the design.

* Pay attention to all details, e.g. the Skills are left aligned in desktop, the tags are wider, the buttons have more padding and another font etc.

Codewise You have a good structure of your project and you’ve used components and props in a good way. Nice usage of useEffect and useRef ⭐

Changes requested Almost there, but needs some additional attention:

* Responsiveness

* Do another take on the design

Other than the project photos comment (question in the above reply), I have made changes according to the other comments regarding responsiveness and following the design.

If there is anything else please let me know.

Thanks!

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

No need to add images if you don't want to, it was just a comment 😄

However:

  • Still got a side-scroll, check responsiveness again
  • Spacings still aren’t according to the design

@christina-baldwin
Copy link
Author

No need to add images if you don't want to, it was just a comment 😄

However:

* Still got a side-scroll, check responsiveness again

* Spacings still aren’t according to the design

I'm not getting a side scroll at all neither on chrome nor firefox so I'm not sure what I need to change, can you check again please?

I'll take another look at the spacings.

@christina-baldwin
Copy link
Author

No need to add images if you don't want to, it was just a comment 😄

However:

* Still got a side-scroll, check responsiveness again

* Spacings still aren’t according to the design

I've also just realised the netlify deploy hadn't been updated so if you were checking that it was the old version, sorry about that!

@christina-baldwin
Copy link
Author

No need to add images if you don't want to, it was just a comment 😄

However:

* Still got a side-scroll, check responsiveness again

* Spacings still aren’t according to the design

Sorry for the multiple comments! I quickly double-checked the spacings again in my code compared to the figma design and those also seemed to match (unless I'm missing something), its possible the issue was that the netlify deploy was the old one before I made changes.

Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

That was it 😅 👍

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.

3 participants