New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MOYO Header #5850

Open

ATkachenkoA wants to merge 8 commits into mate-academy:master from ATkachenkoA:develop1

ATkachenkoA commented Dec 21, 2024 •

edited

Loading

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 ❗️


          add task solution

ff62026

mateacademy-ai-mentor suggested changes

View reviewed changes

mateacademy-ai-mentor left a comment

Thank you for your hard work on this project! 😊 While your code meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

CSS Variable for Blue Color: You need to define and use a CSS variable for the blue color instead of using #00acdc directly. This will enhance maintainability and consistency across your styles.
Flex Gap Property: The task specifies using margin instead of the gap property for flexbox indents. Please adjust the .header__container and .nav__list classes accordingly.
HTML Formatting: Ensure your HTML follows the specified formatting guidelines, such as using 2 spaces for indentation.
Dynamic Active Link Styling: The is-active class should be applied dynamically to reflect the active state accurately. Ensure this is implemented correctly in your HTML.
Navigation Links: The href attributes for navigation links are set to '#', which is not valid. Consider using meaningful URLs or handling the click events with JavaScript if these are placeholders.

Please address these issues to ensure your code fully complies with the task requirements. Once these changes are made, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you have any questions! 🚀

✨ 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

+                          <li class="nav__item">
+                            <a
+                              class="nav__link is-active"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/index.html Outdated

+                          <li class="nav__item">
+                            <a
+                              class="nav__link"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/index.html Outdated

+                          <li class="nav__item">
+                            <a
+                              class="nav__link"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/index.html Outdated

+                            <a
+                              class="nav__link"
+                              data-qa="hover"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/index.html Outdated

+                          <li class="nav__item">
+                            <a
+                              class="nav__link"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/index.html Outdated

+                          <li class="nav__item">
+                            <a
+                              class="nav__link"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/index.html Outdated

+                          <li class="nav__item">
+                            <a
+                              class="nav__link"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/index.html Outdated

+                          <li class="nav__item">
+                            <a
+                              class="nav__link"
+                              href="#"

mateacademy-ai-mentor Dec 21, 2024

The href attribute for the navigation links is set to '#', which is not a valid URL. Consider using a more meaningful URL or a JavaScript function to handle the click event if these are placeholder links.

src/style.css Outdated

Comment on lines 75 to 77

+              .nav__link:nth-child(4) {
+                color: #00acdc;
+              }

mateacademy-ai-mentor Dec 21, 2024

The selector .nav__link:nth-child(4) is used to style the fourth navigation link. This approach is fragile because it relies on the order of elements in the HTML. Consider using a more specific class or data attribute to target this element.

src/style.css Outdated

Comment on lines 54 to 56

+              a.is-active {
+                position: relative;
+                color: #00acdc;

mateacademy-ai-mentor Dec 21, 2024

The class is-active is used to style active links. Ensure that this class is applied dynamically to the correct element in the HTML to reflect the active state accurately.


          add solution

bf0775b

ATkachenkoA requested a review from mateacademy-ai-mentor

December 21, 2024 03:54

mateacademy-ai-mentor approved these changes

View reviewed changes

mateacademy-ai-mentor left a comment

Great job on addressing several issues from the previous review! 🎉 While there are still a few critical points that need attention, your progress is commendable. Here are the key areas to focus on:

CSS Variable for Blue Color: You've defined a CSS variable --header-color for the blue color #00acdc, which is great. However, ensure that this variable is used consistently throughout your code, replacing any direct use of #00acdc.
Flex Gap Property: The task specifies using margin instead of the gap property for flexbox indents. Please adjust the .header__container and .nav__list classes to use margin for spacing.
Dynamic Active Link Styling: The is-active class should be applied dynamically to reflect the active state accurately. Ensure this class is applied based on the current active page.

Your decision to approve this pull request is to help you move forward with your learning journey. Please review the comments to improve your code. Addressing these issues will enhance your code's functionality and maintainability. 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

+                        <ul class="nav__list">
+                          <li class="nav__item">
+                            <a
+                              class="nav__link is-active"

mateacademy-ai-mentor Dec 21, 2024

The is-active class is applied statically to the 'Apple' link. Ensure this class is applied dynamically based on the current active page to reflect the active state accurately.

src/style.css Outdated

+                justify-content: space-between;
+                background-color: white;
+                flex-wrap: wrap;
+                gap: 60px;

mateacademy-ai-mentor Dec 21, 2024

The task specifies using margin instead of the gap property for flexbox indents. Please replace the gap property with appropriate margin settings.

src/style.css Outdated

+              .nav__list {
+                display: flex;
+                flex-wrap: wrap;
+                gap: 20px;

mateacademy-ai-mentor Dec 21, 2024

The task specifies using margin instead of the gap property for flexbox indents. Please replace the gap property with appropriate margin settings.

src/style.css Outdated

+              }
+              .nav__link:nth-child(4) {
+                color: #00acdc;

mateacademy-ai-mentor Dec 21, 2024

Replace the direct use of #00acdc with the CSS variable var(--header-color) for consistency and maintainability.


          add changes

d94f28b

ATkachenkoA requested a review from mateacademy-ai-mentor

December 21, 2024 21:20

ATkachenkoA added 5 commits

December 22, 2024 00:19


          fixed mistakes

7cf0302


          changes

84568ce


          corrected mistakes

e4e6b33


          add fixed solution

178cc00


          solution

dcb9ac7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet