Skip to content

Twitter icon change #522

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

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

Conversation

nitesh2920
Copy link

I've updated the Twitter logo to the new logo

Copy link

netlify bot commented Oct 9, 2024

Deploy Preview for ember-styleguide failed.

Name Link
🔨 Latest commit 2718b08
🔍 Latest deploy log https://app.netlify.com/sites/ember-styleguide/deploys/670909d7b04111000886ca7d

Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

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

Tiny bit more to do 👍

@@ -3,7 +3,7 @@
const socialLinks = [{
title: 'Twitter',
Copy link
Member

Choose a reason for hiding this comment

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

would we not also change the title?

Copy link
Author

@nitesh2920 nitesh2920 Oct 10, 2024

Choose a reason for hiding this comment

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

Okay , I will change the title from Twitter to X.

@@ -3,7 +3,7 @@
const socialLinks = [{
title: 'Twitter',
href: 'http://twitter.com/emberjs',
Copy link
Member

Choose a reason for hiding this comment

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

If we're changing the logo then shouldn't we be linking to x.com/emberjs ?

Copy link
Author

@nitesh2920 nitesh2920 Oct 10, 2024

Choose a reason for hiding this comment

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

okay I will change it.

@@ -3,7 +3,7 @@
const socialLinks = [{
title: 'Twitter',
href: 'http://twitter.com/emberjs',
class: 'twitter-logo',
class: 'x-logo',
label: 'Official Ember Twitter Account'
Copy link
Member

Choose a reason for hiding this comment

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

If we're changing the logo we should probably change Twitter here too

Copy link
Author

Choose a reason for hiding this comment

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

okay I got your point to replace twitter to X , is there anything more ?
does need to change label also ?

@nitesh2920
Copy link
Author

@mansona made the required changes please review it.

@mansona
Copy link
Member

mansona commented Oct 14, 2024

From what I can see this looks good, I just don't feel comfortable merging this until I fix the issues with CI. I'll try to take a look tomorrow 👍

@nitesh2920
Copy link
Author

@mansona Thanks for the update! Looking forward to your feedback tomorrow!

@nitesh2920
Copy link
Author

@mansona any update on this ?

Copy link

netlify bot commented Aug 12, 2025

Deploy Preview for ember-styleguide ready!

Name Link
🔨 Latest commit ec1979d
🔍 Latest deploy log https://app.netlify.com/projects/ember-styleguide/deploys/689af50cd055aa0008dc845f
😎 Deploy Preview https://deploy-preview-522--ember-styleguide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

@BornDeleuze BornDeleuze left a comment

Choose a reason for hiding this comment

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

While running this locally I couldn't get this x icon to load properly. I found that this one worked well though, which will match the website members' social icon for x

'twitter-logo':
'<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1000 1000"><path d="M939.5 227.941q-37 54-90 93v23q0 73-21 145t-64 139q-43 67-103 117t-144 82q-84 32-181 30-151 0-276-81 19 2 43 2 126 0 224-77-59-1-105-36t-64-89q19 3 34 3 24 0 48-6-63-13-104-62t-41-115v-2q38 21 82 23-37-25-59-64t-22-86q0-49 25-91 68 83 164 133t208 55q-5-21-5-41 0-75 53-127t127-53q79 0 132 57 61-12 115-44-21 64-80 100 52-6 104-28z"/></svg>',
'x-logo':
'<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1000 1000"><path d="M 6.9199219 6 L 21.136719 26.726562 L 6.2285156 44 L 9.40625 44 L 22.544922 28.777344 L 32.986328 44 L 43 44 L 28.123047 22.3125 L 42.203125 6 L 39.027344 6 L 26.716797 20.261719 L 16.933594 6 L 6.9199219 6 z"></path></svg>',

Choose a reason for hiding this comment

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

I could not get this svg to render properly in the footer element, but had success with

Suggested change
'<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1000 1000"><path d="M 6.9199219 6 L 21.136719 26.726562 L 6.2285156 44 L 9.40625 44 L 22.544922 28.777344 L 32.986328 44 L 43 44 L 28.123047 22.3125 L 42.203125 6 L 39.027344 6 L 26.716797 20.261719 L 16.933594 6 L 6.9199219 6 z"></path></svg>',
'<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 1000 1000" class="icon"><path d="M100 850 c15 -20 300 -430 305 -435 l2 -3 -150 -175 c-82 -96 -150 -175 -152 -177 l-2 -2 35 0 35 0 133 155 c73 85 133 155 134 155 1 0 48 -70 105 -155 l104 -155 115 0 115 0 -2 3 c-1 2 -72 104 -157 228 -85 124 -155 226 -155 227 0 1 64 75 143 165 79 90 143 164 143 164 0 0 -16 0 -35 0 l-35 0 -125 -145 c-69 -80 -125 -145 -125 -145 0 0 -45 65 -100 145 l-100 145 -115 0 -115 0 2 -3z m215 -77 c90 -128 462 -662 464 -665 1 -2 -10 -3 -52 -3 l-52 0 -110 157 c-60 86 -170 243 -243 349 -73 106 -132 194 -132 196 0 1 22 2 52 2 l52 0 21 -36z"/></svg>',

Choose a reason for hiding this comment

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

Screenshot 2025-08-13 at 9 44 16 PM

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