add task solution #5825
add task solution #5825
Conversation
aleksandra-golebiewska
commented
Dec 16, 2024
aleksandra-golebiewska
commented
- DEMO LINK
- TEST REPORT LINK
- Header height is set in 1 place (for the links)
- Content is vertically centered (for any header height)
- CSS is used to show all letters in Uppercase (don't type them in HTML)
- Logo is an image wrapped with a link
- CSS Variable is used for a blue color
- Pseudo-element is used for a blue line below the active link
- Code follows all the Code Style Rules ❗️
There was a problem hiding this comment.
Thank you for your hard work on this project! 😊 However, there are a few critical issues that need to be addressed before we can approve your pull request:
-
HTML Structure and Semantic Tags:
- The
<img>tag for the logo should be wrapped in a link as per the requirements. - The
hrefattributes in the<a>tags contain spaces and are not valid URLs. Please update them to use valid paths or URLs, such as./video.html.
- The
-
CSS Requirements:
- A CSS variable should be used for the blue color to meet the task requirements.
- Text transformation should be handled using CSS, not by directly using uppercase letters in HTML.
-
General Requirements:
- Ensure the header height is set explicitly in one place as required.
Please make these changes to align with the task requirements. Once these are addressed, your solution will be much closer to completion. Keep up the great work, and don't hesitate to reach out if you need any help! 🚀
✨ 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.
| <ul class="nav__list"> | ||
| <li> | ||
| <a | ||
| href="Apple" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Apple', which is not a valid URL. Consider using a relative path like './apple.html' or an absolute URL.
| </li> | ||
| <li> | ||
| <a | ||
| href="Samsung" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Samsung', which is not a valid URL. Consider using a relative path like './samsung.html' or an absolute URL.
| </li> | ||
| <li> | ||
| <a | ||
| href="Smartphones" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Smartphones', which is not a valid URL. Consider using a relative path like './smartphones.html' or an absolute URL.
| </li> | ||
| <li> | ||
| <a | ||
| href="Laptops&computers" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Laptops&computers', which is not a valid URL. Consider using a relative path like './laptops-computers.html' or an absolute URL.
| </li> | ||
| <li> | ||
| <a | ||
| href="Gadgets" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Gadgets', which is not a valid URL. Consider using a relative path like './gadgets.html' or an absolute URL.
| </li> | ||
| <li> | ||
| <a | ||
| href="Tablets" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Tablets', which is not a valid URL. Consider using a relative path like './tablets.html' or an absolute URL.
| </li> | ||
| <li> | ||
| <a | ||
| href="Photo" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Photo', which is not a valid URL. Consider using a relative path like './photo.html' or an absolute URL.
| </li> | ||
| <li> | ||
| <a | ||
| href="Video" |
There was a problem hiding this comment.
The href attribute should contain a valid URL or path. Currently, it is set to 'Video', which is not a valid URL. Consider using a relative path like './video.html' or an absolute URL.
There was a problem hiding this comment.
Great progress on your project! 🎉 You've successfully addressed several key requirements, such as using CSS variables for colors and handling text transformation with CSS. However, there are a few areas that need attention:
-
HTML Structure and Semantic Tags:
- Ensure that all
hrefattributes in your<a>tags are valid URLs or paths. For example, use"#apple"instead of"Apple"for placeholders. - The
hrefattribute for the logo link should be updated to a valid path or URL, like"#"if it's a placeholder.
- Ensure that all
-
CSS Requirements:
- You've done well by using CSS for text transformation and setting the header height in one place.
These improvements will enhance the functionality and maintainability of your code. Keep up the good work, and remember to address these issues in future projects to avoid similar problems. Your dedication to learning and improving is commendable! 💪
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -18,5 +31,82 @@ | |||
| </head> | |||
| <body> | |||
| <h1>Moyo header</h1> | |||
| <header> | |||
| <a href="logo" class="images"> | |||
There was a problem hiding this comment.
The href attribute for the logo link should be a valid URL or path. Consider using a relative path like ./index.html if this is meant to link back to the homepage.
| <ul class="nav__list"> | ||
| <li> | ||
| <a | ||
| href="Apple" |
There was a problem hiding this comment.
The href attribute contains an invalid URL. Consider using a valid path or URL, such as ./apple.html.
| </li> | ||
| <li> | ||
| <a | ||
| href="Samsung" |
There was a problem hiding this comment.
The href attribute contains an invalid URL. Consider using a valid path or URL, such as ./samsung.html.
| </li> | ||
| <li> | ||
| <a | ||
| href="Smartphones" |
There was a problem hiding this comment.
The href attribute contains an invalid URL. Consider using a valid path or URL, such as ./smartphones.html.
| </li> | ||
| <li> | ||
| <a | ||
| href="Laptops&computers" |
There was a problem hiding this comment.
The href attribute contains an invalid URL. Consider using a valid path or URL, such as ./laptops-computers.html.
| href="Photo" | ||
| class="nav__link" | ||
| > | ||
| PHOTO |
There was a problem hiding this comment.
Text transformation should be handled using CSS. Consider using CSS to transform text to uppercase instead of directly using uppercase letters in HTML.
| href="Video" | ||
| class="nav__link" | ||
| > | ||
| VIDEO |
There was a problem hiding this comment.
Text transformation should be handled using CSS. Consider using CSS to transform text to uppercase instead of directly using uppercase letters in HTML.
| margin: 0; | ||
|
|
||
| --text-color-second: #00acdc; |
There was a problem hiding this comment.
Good job using a CSS variable for the blue color. This aligns with the task requirement to use CSS variables.
| justify-content: space-between; | ||
| box-sizing: border-box; | ||
| width: 100%; | ||
| text-transform: uppercase; |
There was a problem hiding this comment.
Text transformation is correctly handled using CSS with text-transform: uppercase;. This is in line with the task requirements.
| box-sizing: border-box; | ||
| width: 100%; | ||
| text-transform: uppercase; | ||
| height: 60px; |
There was a problem hiding this comment.
The header height is set explicitly, which meets the task requirement to ensure the header height is set in one place.
