London | 26-ITP-May | Chun Yan Wong | Sprint 2 | Wireframe#1376
London | 26-ITP-May | Chun Yan Wong | Sprint 2 | Wireframe#1376cywong-dev wants to merge 2 commits into
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
cywong-dev
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
-
According to https://validator.w3.org/, there are errors in your code. Can you fix them?
-
To better match the layout in the wireframe, can you make the title and the "Read More" links in articles 2 and 3 fully aligned horizontally?
- Note: If you have not yet enabled "Format on save" on VS Code, consider following this guide to enable VS Code's
"Format on Save" option or to use its "Format Document" command to ensure your code is always consistently formatted.
| </header> | ||
| <main> | ||
| <article> | ||
| <img src="readme.svg" alt="" /> |
There was a problem hiding this comment.
Providing meaningful alt text is important because it ensures images are accessible to people using screen readers and also helps when images
fail to load, giving users essential context about the content or function of the image.
Can you give a more descriptive alt value to all images?
| <ul> | ||
| <li> | ||
| What is the purpose of a README file? | ||
| </li> | ||
| <li> | ||
| What is the purpose of a wireframe? | ||
| </li> | ||
| <li> | ||
| What is a branch in Git? | ||
| </li> | ||
| </ul> |
There was a problem hiding this comment.
The wireframe has a "page title" instead of an unordered list. In addition, in the wireframe the page title and the text beneath it are centered.
| <p> | ||
| By ChunyanWong HOMEWORK FOR WIREFRAME SOLUTION | ||
| </p> |
There was a problem hiding this comment.
-
To better match the wireframe, can you keep the footer content centered?
-
Currently the footer content and the article content could overlap. Can you find a way to prevent it from happening?
There was a problem hiding this comment.
There are two main selectors in this file. Consider merging the properties from both selectors into a single main rule to make them easier to find and maintain.
cjyuan
added
Reviewed
2 participants
Learners, PR Template
Self checklist
Changelist
I completed the changed based on the requirement