London | 26-ITP-Jan | Xuanming Hu | Sprint 1 | Wireframe#1048
London | 26-ITP-Jan | Xuanming Hu | Sprint 1 | Wireframe#1048Samual-Hu wants to merge 3 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.
This comment has been minimized.
This comment has been minimized.
4 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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cjyuan
left a comment
There was a problem hiding this comment.
I expected updates to both index.html and style.css so that the page elements are styled to match the wireframe.png image.
cjyuan
left a comment
There was a problem hiding this comment.
-
Code is free of syntax error. Well done.
-
When a wireframe is provided, our implementation should closely reflect its appearance and layout to ensure consistency with design expectations. You're off to a solid start. To better align with the wireframe, can you
- Set the images in articles 2 and 3 to have identical height.
- (Optional challenge) Align the positions of the title and the "Read more" link between articles 2 and 3.
-
There is a gap between the left border of the footer and the viewport, but not on the right (see pic). That means the footer is not quite centered. Can you make the footer "more center"?
| <main> | ||
|
|
||
| <!-- Article 1: Questions 1 --> | ||
| <article> |
There was a problem hiding this comment.
Can you improve the indentation of the code?
Consider using VSCode's "Format Document" feature to auto format the code for better readability and consistency. To use the feature, right-click inside the code editor and select the option.
|
|
||
| <!-- Article 2: Question 2 --> | ||
| <article> | ||
| <img src="https://images.prismic.io/prismic-main/Zoa6gh5LeNNTwzHJ_web_design_wireframe.jpeg?auto=format,compress" alt="Wireframe example image" /> |
There was a problem hiding this comment.
What's your rationale for using this URL:
https://images.prismic.io/prismic-main/Zoa6gh5LeNNTwzHJ_web_design_wireframe.jpeg?auto=format,compress
instead of this
https://images.prismic.io/prismic-main/Zoa6gh5LeNNTwzHJ_web_design_wireframe.jpeg
?
They both "seem" to retrieve the same image.
| footer { | ||
| position: fixed; | ||
| bottom: 0; | ||
| text-align: center; | ||
| font-weight: 500; | ||
| border: solid black 2px; | ||
| width: 100%; | ||
| background-color: white; | ||
| color: black; | ||
| } No newline at end of file |
There was a problem hiding this comment.
You can also use VSCode "Format Document" feature to auto format CSS code.
|
Regarding the "Changelist" section of your PR description. Since you have made more changes than just "Changed the header description", can you update the Changelist section accordingly? |
|
Changes to the code look good. Can you also address this request:
Please note that in CYF courses, the recommended way to inform the reviewer of your changes is to do both of the following:
|
|
Hi, @cjyuan , I have addressed your comment |
|
The Changelist section is supposed to describe the difference between this PR branch and CYF's "Addressed issues in comments" does not quite describe what you have implemented. Can you give more details? |
|
Hi, @cjyuan , I have updated the description with the specific changes. |
|
All good now. Well done. |
|
Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it. |

Learners, PR Template
Self checklist
Changelist
Questions
None for now.