Sheffield | 26-ITP-JAN | Grant Riches | Sprint 1 | Wireframe#1049
Sheffield | 26-ITP-JAN | Grant Riches | Sprint 1 | Wireframe#1049grantriches-byte wants to merge 12 commits intoCodeYourFuture:mainfrom
Conversation
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
6 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
grantriches-byte
changed the title
…ackgound changed and padding added.
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
This PR does not have a "Needs Review" label, but I assume it is ready to be reviewed.
Here are my feedback:
-
According to https://validator.w3.org/, there are two warnings in your code. Can you address them?
-
In the wireframe image, the Read Me links have a border. Can you add a border to all three Read Me links?
-
Optional Change: If you are up to some more challenge, you can try laying out the Read Me links in articles 2 and 3 so that they align horizontally nicely regardless of the length of the articles.
Don't forget to add the "Needs review" label to this PR when you have made all the changes.
| A branch in Git is like a separate workspace where you can make changes and try new ideas without affecting the main project. | ||
| Creating a branch allows you to make changes without affecting the original version. | ||
| </p> | ||
| <a href="https://www.w3schools.com/git/git_branch.asp?_gl=1*1pmlokl*_ga*NzI4ODc3OTYyLjE3NjkxMTc0MzI.*_ga_9YNMTB56NB*czE3NjkxMTc0MzEkbzEkZzEkdDE3NjkxMTc0MzIkajU5JGwwJGgw">Read more about Git branches</a> |
There was a problem hiding this comment.
Part of the URL in the link on line 45 is probably not needed.
Can you use an AI tool to find out which part of the URL can be deleted to produce a cleaner URL?
| --space: clamp(6px, 6px + 2vw, 15px); | ||
| --line: 1px solid; | ||
| --container: 1280px; | ||
| V/* Reset */ |
There was a problem hiding this comment.
Looks like there is a typo on line 1.
cjyuan
added
the
Reviewed
grantriches-byte
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
grantriches-byte
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
grantriches-byte
added
the
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
- The footer content and the article content can overlap (see pic). Can you find a way to prevent this from happening?
-
When a wireframe is provided, our implementation should closely reflect its appearance and layout to ensure consistency with design expectations. To better align with the wireframe, can you
- Introduce a border to all three articles?
- Make the image span the full width of the articles?
-
According to https://validator.w3.org/, there is a warning in your code. Can you fix it?
cjyuan
removed
the
Needs Review
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |
3 participants
Learners, PR Template
Self checklist
Changelist
I have completed all steps of the Wireframe task.