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 #5796

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

Develop #5796

wants to merge 3 commits into from

Conversation

VM93
Copy link

@VM93 VM93 commented Dec 9, 2024

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! 😊 While you've made a great effort, there are a few critical issues that need to be addressed before we can approve the pull request.

  1. Image Path Issue: The use of backslashes (\) in the image path can cause problems in web environments. Please update these to forward slashes (/) to ensure compatibility across different platforms.

  2. CSS Universal Selector: The use of the universal selector * can impact performance and is discouraged. Please replace it with more specific selectors.

  3. CSS Styling with Tag Names: Avoid using tag names like ul and li for styling. Instead, use classes or other selectors as specified in the task requirements.

  4. CSS Fallback Fonts: Ensure you include fallback fonts in your font-family declarations to maintain consistent typography across different browsers.

  5. CSS Variable for Blue Color: Implement a CSS variable for the blue color as required by the task.

  6. Uppercase Text and Vertical Centering: Ensure all text is displayed in uppercase and content is vertically centered as specified.

  7. Active Link Styling: Verify that the pseudo-element for the active link aligns with the design specifications.

  8. Nav Links Styling: Ensure nav links have no padding and centered text as required.

Please address these issues to align with the task requirements. Keep up the good 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.

src/index.html Outdated
class="img"
>
<img
src="images\logo.png"

Choose a reason for hiding this comment

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

The file path for the image source uses a backslash ('\') which is typically used in Windows file paths. For web URLs, it's recommended to use a forward slash ('/') to ensure compatibility across different environments.

@VM93 VM93 closed this Dec 10, 2024
@VM93 VM93 reopened this Dec 10, 2024
@VM93 VM93 closed this Dec 10, 2024
@VM93 VM93 reopened this Dec 10, 2024
Copy link

@maxim2310 maxim2310 left a comment

Choose a reason for hiding this comment

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

Good work

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.

3 participants