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

Replaced old twitter icon with new icon #1124

Merged
merged 4 commits into from
Feb 6, 2024

Conversation

antriksh-9
Copy link
Contributor

Linked Issue

Closes #1112.

Description

On the website under navbar section the twitter logo was old one.
I changed it to the new logo.

Screenshot 2024-01-30 230417

Code of Conduct

By submitting this pull request, you agree to follow our Code of Conduct

@antriksh-9 antriksh-9 requested a review from a team as a code owner January 30, 2024 17:34
Copy link

netlify bot commented Jan 30, 2024

👷 Deploy Preview for virtual-coffee-io processing.

Name Link
🔨 Latest commit 4776259
🔍 Latest deploy log https://app.netlify.com/sites/virtual-coffee-io/deploys/65bcf52e66770b000815a1eb

xmlns="http://www.w3.org/2000/svg"
width="24"
height="24"
viewBox="0 0 512 512"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @antriksh-9,
Is there any reason to remove the role="img" attribute and change the viewBox dimension?

xmlns="http://www.w3.org/2000/svg"
width="24"
height="24"
viewBox="0 0 512 512"
>
<title>Twitter</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

You might also want to change this from "Twitter" to "X".

Suggested change
<title>Twitter</title>
<title>X</title>

@antriksh-9
Copy link
Contributor Author

Hey @adiati98 I have made the necessary changes.

Copy link
Member

@danieltott danieltott left a comment

Choose a reason for hiding this comment

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

Check my comment - we definitely needed the viewbox change - now the icon isn't showing up!

image

@@ -82,12 +82,12 @@ export default function Nav() {
<svg
role="img"
viewBox="0 0 24 24"
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we needed that viewbox change - not sure what the problem there was...

If you update that we can get this up!

Suggested change
viewBox="0 0 24 24"
viewBox="0 0 512 512"

@antriksh-9
Copy link
Contributor Author

hey @danieltott I have updated the file.

Copy link
Member

@danieltott danieltott left a comment

Choose a reason for hiding this comment

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

Looks good - thanks @antriksh-9!

@danieltott danieltott merged commit 548330a into Virtual-Coffee:main Feb 6, 2024
6 checks passed
@antriksh-9 antriksh-9 deleted the twitter branch March 6, 2024 11:42
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.

Replace old twitter icon with new icon
3 participants