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

Increase size of Kanvas icon for better visibility #6070

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

Conversation

Tharanishwaran
Copy link
Member

Description

This PR fixes #

Notes for Reviewers

This PR increases the size of Kanvas icon on the layer5.io/kanvas page to enhance visual clarity.

Before :

Screenshot from 2024-11-10 13-17-55

After :

Screenshot from 2024-11-10 21-31-12

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Tharanishwaran <[email protected]>
@l5io
Copy link
Contributor

l5io commented Nov 10, 2024

🚀 Preview for commit be1a4d4 at: https://6730e2adaeb49ae29e91292c--layer5.netlify.app

@vishalvivekm
Copy link
Member

@Tharanishwaran
image

@Tharanishwaran
Copy link
Member Author

@Tharanishwaran
Copy link
Member Author

@vishalvivekm, are there any changes needed in this PR?

@vishalvivekm
Copy link
Member

@NishantSinghhhhh ^^

@vishalvivekm
Copy link
Member

@Tharanishwaran please check the logo in different screen width, if you would

@Tharanishwaran
Copy link
Member Author

Okay @vishalvivekm , I will test it on different screens and commit the changes afterward.

@Tharanishwaran
Copy link
Member Author

Tharanishwaran commented Nov 10, 2024

@vishalvivekm , I made the changes and tested the logo on all screen sizes. Here’s a screenshot of how the logo looks on the last screen size.
If it looks good to you, I’ll go ahead and commit the changes.

Screenshot from 2024-11-11 01-52-28

@vishalvivekm
Copy link
Member

@Tharanishwaran Thank you for your contribution!
Let's discuss this during the website call on Monday at 5:30 PM IST

Please add it as an agenda item to the meeting minutes.

@l5io
Copy link
Contributor

l5io commented Nov 12, 2024

🚀 Preview for commit 397c354 at: https://6733895af175d70879002fc7--layer5.netlify.app

@vishalvivekm
Copy link
Member

@Tharanishwaran
can we discuss this in today's websites call?

@Tharanishwaran
Copy link
Member Author

@Tharanishwaran can we discuss this in today's websites call?

@vishalvivekm Yes, sure

@l5io
Copy link
Contributor

l5io commented Nov 23, 2024

🚀 Preview for commit dfa511e at: https://6742031ee47b0b66e5fcc5e2--layer5.netlify.app

@l5io
Copy link
Contributor

l5io commented Nov 23, 2024

🚀 Preview for commit 6aae58f at: https://67421eeb5741b292e251f3c8--layer5.netlify.app

}
border-radius: 0% 85% 0% 0% / 0% 40% 0% 0% ;
}
@media screen and (max-width: 210px) {
Copy link
Member

Choose a reason for hiding this comment

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

this overuse of media queries is not a good practice. This will become hard to maintain and when something changes around this component we again had to rewrite its styles. Use better css, like use flex, max width etc and make more dynamic rather than fixing on each screen width.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, @sudhanshutech I'll refactor the CSS using more dynamic techniques like flex and max-width instead of relying on media queries.

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

Successfully merging this pull request may close these issues.

5 participants