Conversation
There was a problem hiding this comment.
status: require changes ♻️
Great job you have done really well 👍 💯 👏, but you need to add push more in this project to get it approved 🏁.
-
please make all the of the article titles bold, and also all images and titles should be wrapped by a link tag.
-
the login button on the navbar should be white not black.
-
you ave forgot to add a whole section:
-
the articles should start on the same line:
| Clone | Original |
|---|---|
 |
 |
same here:
| Clone | Original |
|---|---|
 |
 |
Please make all the required changes before you submit another code review.
Happy Coding 🎩
|
changed color of the login button |
There was a problem hiding this comment.
Changes Requested
Hello! 👋
Awesome job so far!! 👍 you're a few steps away to get this project approved!
Required Changes ♻️
- Pull Request has a meaningful title and a brief description (screenshot from Pathwright), change the PR to have more information in what you implemented in this project
- on small screens, 'the debate' and 'opinion' sections comes after 'more stories' section (link to page)
Optional
- Your have some warning in the HTML Validator, please fix it
please make the appropriate changes and ask for another code review ✔️
Happy Coding!
Best,
Aziz Mejri
contact me on twitter or slack (@SpaYco) if you need any help
m-anwar97
changed the title
|
changed pull request title |
There was a problem hiding this comment.
Status: Changes Required ♻️
Hey 👍
You did a great job on this project, and it is almost finished. However, I think we could improve in a couple of areas yet. Since most cases were not pointed in the first review, I'll mark them as optional.
Check the inline comments for the optional changes. What is mandatory is:
- Validate your HTML here
As stated above, the project looks really good.
Please make the required changes and resubmit for a new code review once you are ready
Good Luck and Happy Coding 🙏
P.S. If you have any questions about my review or need help, you can message me on slack (@ Expedito Andrade).:smile:
|
The heading warnings are not valid for my project's case as this is a replica, adding headings will make the page look different from the original |
|
fixed the two optional requirments |
There was a problem hiding this comment.
status: require changes ♻️
Hi, it's me again ✋, great job so far, thank you for making the required changes, you still need to work only on one thing.
- please fix the warnings on the HTML validator, as mentioned by the previous TSE's. I know that by adding headings will ruin your page but you can also use display: none; to hide the headings as we did in this project.
please make the required changes before you submit another code review.
Happy Coding 🎩
|
fixed all warnings |
BrittanyBlake
left a comment
BrittanyBlake
left a comment
There was a problem hiding this comment.
Status: Approved ✅
Hi Team! 👋
Great job implementing the requested changes!! 🎖️ It's a 👍 from me! Go ahead and merge this pull request and keep up the good work 😄
Good Luck and Happy Coding! 🍀 👩🏾💻
5 participants
This project is a clone of Newsweek ( www.newsweek.com ).