Birmingham | ITP-Jan | Ahmad Roman Sanaye | Sprint 1 | Wireframe #940
Conversation
This comment has been minimized.
This comment has been minimized.
✅ Deploy Preview for cyf-onboarding-module ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
RomanSanaye
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
bot
removed
the
Needs Review
RomanSanaye
added
the
Needs Review
RomanSanaye
changed the title
jenny-alexander
added
Review in progress
jenny-alexander
left a comment
jenny-alexander
left a comment
There was a problem hiding this comment.
Hi @RomanSanaye! Thanks for submitting your PR for the wireframe exercise. I reviewed your submission and outlined a few areas to make changes in order to strengthen your code a bit.
Wireframe/index.html
Outdated
| <article> | ||
| <img src="placeholder.svg" alt="" /> | ||
| <h2>Title</h2> | ||
| <img src="placeholder.svg" alt="README file illustration" /> |
There was a problem hiding this comment.
Can you please change the placeholder image for an image related to the article?
Wireframe/index.html
Outdated
| </article> | ||
|
|
||
| <article> | ||
| <img src="placeholder.svg" alt="Wireframe layout example" /> |
There was a problem hiding this comment.
Can you please change the placeholder image for an image related to the article?
Wireframe/index.html
Outdated
| </article> | ||
|
|
||
| <article> | ||
| <img src="placeholder.svg" alt="Git branch diagram" /> |
There was a problem hiding this comment.
Can you please change the placeholder image for an image related to the article?
Wireframe/index.html
Outdated
| how to use it. It is usually the first file users read. | ||
| </p> | ||
| <a href="">Read more</a> | ||
| <a href="#">Read more</a> |
There was a problem hiding this comment.
The Read more button should bring the user to an article about README files.
Wireframe/index.html
Outdated
| A wireframe shows the basic structure of a webpage before design and | ||
| development begin. It focuses on layout and content placement. | ||
| </p> | ||
| <a href="#">Read more</a> |
There was a problem hiding this comment.
The Read more button should bring the user to an article about wireframes.
Wireframe/index.html
Outdated
| A Git branch allows developers to work on new features separately | ||
| without affecting the main project code. | ||
| </p> | ||
| <a href="#">Read more</a> |
There was a problem hiding this comment.
The Read more button should bring the user to an article about git branches.
jenny-alexander
added
Reviewed
Hi! Thanks a lot for reviewing my PR and sharing your feedback. I appreciate the detailed suggestions and will apply the recommended changes to strengthen the code. I’ll update the PR shortly. # |
|
Hi @jenny-alexander, and thanks for your feedback! 🙏 |
|
Hey @RomanSanaye - you made some solid changes to your wireframe page. It is visually appealing and the I will mark this as complete. In the future, here a few more tips for you:
|
jenny-alexander
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
This PR implements the wireframe coursework using semantic HTML and CSS.
The page includes a header with title and description, three articles explaining
README files, wireframes, and Git branches, and a fixed footer.
The layout matches the provided wireframe and passes Lighthouse accessibility checks.
Questions
No questions at the moment. Feedback is welcome.