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

Closed
wants to merge 15 commits into from
Closed

Develop #5360

wants to merge 15 commits into from

Conversation

Krrampuss
Copy link

@Krrampuss Krrampuss commented Sep 14, 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.

Hey there! 👋 Great job on the progress so far! We just need to address a few important points before we can approve the Pull Request. Specifically, let's work on the formatting and semantics of the code. Correctly structuring the href attributes, improving readability with proper indentation and spacing, and using more meaningful class names will significantly enhance the quality of your code. Keep up the good work, and with these changes, your code will be in tip-top shape! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/style.css Outdated Show resolved Hide resolved
Copy link

@BudnikOleksii BudnikOleksii 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 👍
Left a few comments. Remove duplicated files, also fix links to demo and test report as now they are just point to your repository instead

src/index.html Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/index.html Outdated Show resolved Hide resolved
src/style.css Outdated Show resolved Hide resolved
src/style.css Outdated Show resolved Hide resolved
style.css Outdated Show resolved Hide resolved
Copy link

@BudnikOleksii BudnikOleksii left a comment

Choose a reason for hiding this comment

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

Almost done, just a few moments should be fixed.
And links are still incorrect, they should be with github.io instead of github.com, check the examples in readme:
image

src/index.html Outdated Show resolved Hide resolved
src/style.css Outdated Show resolved Hide resolved
src/style.css Outdated Show resolved Hide resolved
@Krrampuss Krrampuss closed this Sep 18, 2024
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