add header #5740
add header #5740
Conversation
BTakhaiev
commented
Nov 27, 2024
•
BTakhaiev
commented
- DEMO LINK
- TEST REPORT LINK
- [ x] Header height is set in 1 place (for the links)
- [ x] Content is vertically centered (for any header height)
- [ x] CSS is used to show all letters in Uppercase (don't type them in HTML)
- [ x] Logo is an image wrapped with a link
- [ x] CSS Variable is used for a blue color
- [ x] Pseudo-element is used for a blue line below the active link
- [ x] Code follows all the Code Style Rules ❗️
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. These include fixing the incorrect color values in the CSS, ensuring all CSS rules are properly closed, and implementing a CSS variable for the blue color as required. Additionally, the HTML file needs adjustments for the logo URL and alt attribute. Please make these changes to meet the core requirements. Keep up the great effort, and feel free 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.
src/index.html
Outdated
| </head> | ||
| <body> | ||
| <h1>Moyo header</h1> | ||
| <header class="header"> | ||
| <a href="http://127.0.0.1:5500/src/index.html"><img class="img" src="images/logo.png" alt="logo"></a> |
There was a problem hiding this comment.
Consider using a relative URL for the logo image source to ensure it works correctly regardless of the server or domain where the HTML file is hosted. This will make the path more flexible and adaptable to different environments.
src/index.html
Outdated
| </head> | ||
| <body> | ||
| <h1>Moyo header</h1> | ||
| <header class="header"> | ||
| <a href="http://127.0.0.1:5500/src/index.html"><img class="img" src="images/logo.png" alt="logo"></a> |
There was a problem hiding this comment.
It's recommended to use 'https' instead of 'http' for the link to improve security, even for local development. This ensures consistency with other secure links in the document.
src/style.css
Outdated
| .header { | ||
| display: flex; | ||
| width: 1200px; | ||
| background-color: fff; |
There was a problem hiding this comment.
The color value 'fff' is missing the '#' symbol. It should be '#fff' to correctly represent the white color in hexadecimal format.
| width: 1200px; | ||
| background-color: fff; | ||
| justify-content: space-between; | ||
|
|
There was a problem hiding this comment.
There is a missing closing bracket '}' for the '.header' class. Ensure that each CSS rule set is properly closed to avoid unexpected behavior.
src/style.css
Outdated
| width: 100%; | ||
| height: 4px; | ||
| border-radius: 8px; | ||
| background-color: b#00ACDC; |
There was a problem hiding this comment.
The color value 'b#00ACDC' is incorrect. It should be '#00ACDC' without the 'b' prefix to correctly represent the color in hexadecimal format.
There was a problem hiding this comment.
Great progress! 🎉 While your solution is approved to help you move forward with your learning, there are a few areas to improve.
-
CSS Variable for Blue Color: Define a CSS variable for the blue color (
#00acdc) and use it consistently throughout your CSS file. This will enhance maintainability and meet project requirements. -
Logo URL and Alt Attribute: Ensure the
srcattribute of the logo points to the correct image path. Additionally, make thealtattribute more descriptive for better accessibility, such as "Company Logo" or "Moyo Logo". -
Header Width: Adjust the header to stretch across the full page width instead of a fixed
1200pxto ensure responsiveness. -
Anchor Tag URL: Update the
hrefattribute in the anchor tag to point to the actual homepage URL instead of a local server URL. Use a relative path or the correct URL for deployment.
Addressing these issues will not only meet the project requirements but also improve the overall quality 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! ✨
| </head> | ||
| <body> | ||
| <h1>Moyo header</h1> | ||
| <header class="header"> | ||
| <a href="https://127.0.0.1:5500/src/index.html"> |
There was a problem hiding this comment.
The href attribute in the anchor tag should point to the actual homepage URL rather than a local server URL like https://127.0.0.1:5500/src/index.html. Consider using a relative path or the correct URL for deployment.
| <a href="https://127.0.0.1:5500/src/index.html"> | ||
| <img | ||
| class="img" | ||
| src="images/logo.png" |
There was a problem hiding this comment.
Ensure the src attribute points to the correct path of the logo image. If images/logo.png is not the correct path, please update it accordingly.
| <img | ||
| class="img" | ||
| src="images/logo.png" | ||
| alt="logo" |
There was a problem hiding this comment.
The alt attribute should be more descriptive to improve accessibility. Instead of just 'logo', consider something like 'Company Logo' or 'Moyo Logo'.
| } | ||
|
|
||
| .blue { | ||
| color: #00acdc; |
There was a problem hiding this comment.
The color value #00acdc should be defined as a CSS variable as per the project requirements. Consider adding a variable like --blue-color: #00acdc; at the top of your CSS and using it here.
| } | ||
|
|
||
| a:hover { | ||
| color: #00acdc; |
There was a problem hiding this comment.
The color value #00acdc should be replaced with the CSS variable for the blue color. Ensure consistency by using the variable throughout the CSS file.
| } | ||
|
|
||
| .is-active { | ||
| color: #00acdc; |
There was a problem hiding this comment.
The color value #00acdc should be replaced with the CSS variable for the blue color. Ensure consistency by using the variable throughout the CSS file.
| width: 100%; | ||
| height: 4px; | ||
| border-radius: 8px; | ||
| background-color: #00ACDC; |
There was a problem hiding this comment.
The background-color value #00ACDC should be replaced with the CSS variable for the blue color. Ensure consistency by using the variable throughout the CSS file.
