Develop #5812
Develop #5812
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 There are a few critical issues that need to be addressed before we can approve the pull request. The HTML structure is missing some required attributes, such as the href for the logo link and the data-qa="hover" attribute for the "LAPTOPS & COMPUTERS" link. Additionally, there are several typographical errors in the navigation menu that need correction. The CSS has some issues with unsupported properties and positioning that could affect the layout. Please address these points to ensure the functionality and requirements are fully met. Keep up the great effort, and I'm sure you'll have it ready in no time! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
src/index.html
Outdated
| <a class="nav_a blue is-active">APPLE</a> | ||
| </li> | ||
| <li class="nav_li"> | ||
| <a class="nav_a">SAMSNG</a> |
There was a problem hiding this comment.
There is a typographical error in the navigation item 'SAMSNG'. It should be 'SAMSUNG'.
src/index.html
Outdated
| <a class="nav_a">GADGETS</a> | ||
| </li> | ||
| <li class="nav_li"> | ||
| <a class="nav_a">TALETS</a> |
There was a problem hiding this comment.
There is a typographical error in the navigation item 'TALETS'. It should be 'TABLETS'.
src/style.css
Outdated
| width: 1200px; | ||
| margin: 0 auto; | ||
| min-width: 500px; | ||
| max-width: 960px; |
There was a problem hiding this comment.
There is an inconsistency in the width settings for the .header class. The width is set to 1200px, but the max-width is set to 960px. This could lead to unexpected layout behavior. Consider revising these values to ensure they align with your design intentions.
There was a problem hiding this comment.
Great job on addressing the previous issues! 🎉 Your HTML and CSS files have been significantly improved, and the code now meets the task requirements. Here are some points to consider for further refinement:
- HTML: Ensure the
hrefattribute for the logo link correctly points to the homepage URL, asindex.htmlmight not be the intended destination. - CSS: Avoid using
!importantunless necessary, as it can cause specificity issues. Also, check the.a::first-lineselector to ensure it targets the correct element. - Typographical Errors: Ensure all navigation menu items are in the expected format, such as 'VIDEO', 'PHOTO', 'TABLETS', etc.
These improvements will enhance the functionality and maintainability of your code. Keep up the great work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
src/index.html
Outdated
| <h1>Moyo header</h1> | ||
| <header class="header"> | ||
| <a | ||
| href="index.html" |
There was a problem hiding this comment.
The href attribute for the logo link is set to index.html. Ensure this is correct, as typically the logo links to the homepage, which might be a different URL.
src/index.html
Outdated
| href="index.html" | ||
| class="nav_a is-active" | ||
| > | ||
| Apple |
There was a problem hiding this comment.
Typographical error: 'Apple' should be 'APPLE' to match the expected format.
src/index.html
Outdated
| href="index.html" | ||
| class="nav_a" | ||
| > | ||
| Samsung |
There was a problem hiding this comment.
Typographical error: 'Samsung' should be 'SAMSUNG' to match the expected format.
src/index.html
Outdated
| href="index.html" | ||
| class="nav_a" | ||
| > | ||
| Smartphones |
There was a problem hiding this comment.
Typographical error: 'Smartphones' should be 'SMARTPHONES' to match the expected format.
src/index.html
Outdated
| class="nav_a blue" | ||
| data-qa="hover" | ||
| > | ||
| Laptops & Computers |
There was a problem hiding this comment.
Typographical error: 'Laptops & Computers' should be 'LAPTOPS & COMPUTERS' to match the expected format.
src/index.html
Outdated
| href="index.html" | ||
| class="nav_a" | ||
| > | ||
| Tablets |
There was a problem hiding this comment.
Typographical error: 'Tablets' should be 'TABLETS' to match the expected format.
src/index.html
Outdated
| href="index.html" | ||
| class="nav_a" | ||
| > | ||
| Photo |
There was a problem hiding this comment.
Typographical error: 'Photo' should be 'PHOTO' to match the expected format.
src/index.html
Outdated
| href="index.html" | ||
| class="nav_a" | ||
| > | ||
| Video |
There was a problem hiding this comment.
Typographical error: 'Video' should be 'VIDEO' to match the expected format.
src/style.css
Outdated
| text-decoration: none !important; | ||
| } | ||
|
|
||
| .a::first-line { |
There was a problem hiding this comment.
The selector .a::first-line seems incorrect. Ensure that .a is a valid class in your HTML, or adjust the selector to target the correct element.
src/style.css
Outdated
|
|
||
| .blue:hover { | ||
| color: var(--for-link); | ||
| text-decoration: none !important; |
There was a problem hiding this comment.
Avoid using !important unless absolutely necessary, as it can cause specificity issues and make the CSS harder to maintain.
DEMO LINK
TEST REPORT LINK