Skip to content
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.

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

Develop #5875

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Develop #5875

wants to merge 5 commits into from

Conversation

Ludson130
Copy link

Preciso de ajuda.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 correcting the typo in the rel attribute, ensuring the href attributes are properly defined, and fixing the incorrect font-weight value. Additionally, the misuse of justify-content and align-items on the body element should be corrected. Once these issues are resolved, your code will be in great shape! Keep up the great effort! 💪

✨ 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
http-equiv="X-UA-Compatible"
content="ie=edge"
<title>Document</title>
<link rel="styleet" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the rel attribute value. It should be stylesheet instead of styleet.

src/index.html Outdated
<nav>
<ul class="menu">
<li class="element logo">
<a href=" ">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute in this anchor tag is empty. Consider providing a valid URL or removing the attribute if it's not needed.

src/style.css Outdated
Comment on lines 9 to 10
justify-content: center;
align-items: center;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The justify-content and align-items properties are typically used with flexbox or grid containers. Since body is not set as a flex or grid container, these properties will have no effect.

src/style.css Outdated
text-decoration: none;
height: 15px;
font-size: 12px;
font-weight: 500px;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The font-weight property should be a numeric value (e.g., 400, 500, 700) or one of the keyword values (e.g., normal, bold). The current value '500px' is incorrect and should be corrected.

@Ludson130
Copy link
Author

Preciso de ajuda.

Copy link
Author

@Ludson130 Ludson130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correções primárias.

Copy link
Author

@Ludson130 Ludson130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final

Copy link
Author

@Ludson130 Ludson130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


Great job on addressing the previous issues in your code! 🎉 Your HTML and CSS files have seen significant improvements, and the code now meets the task requirements and constraints. However, there are still a few areas to refine:

  • The href attributes in your HTML are currently set to #, which is a placeholder. Please replace these with the actual URLs for the links such as 'Video', 'Photo', 'Tablets', 'Gadgets', 'Laptops & Computers', 'Smartphones', 'Samsung', 'Apple', and the logo link.

  • Ensure that the justify-content and align-items properties are used appropriately within the correct elements, as previously mentioned.

Your efforts are commendable, and this approval is to help you move forward with your learning. Please review the comments to further improve your code. Addressing these issues will enhance the functionality and user experience of your project. 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! ✨

<header class="header">
<a
class="logo"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL where the logo link should point.

<li class="element">
<a
class="link is-active"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Apple' link.

<li class="element">
<a
class="link"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Samsung' link.

<li class="element">
<a
class="link"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Smartphones' link.


<li class="element">
<a
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Laptops & Computers' link.

<li class="element">
<a
class="link"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Gadgets' link.

<li class="element">
<a
class="link"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Tablets' link.

<li class="element">
<a
class="link"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Photo' link.

<li class="element">
<a
class="link"
href="#"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The href attribute is set to #, which is a placeholder. Please replace it with the actual URL for the 'Video' link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants