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

Old logo for X (formally Twitter) #2775

Closed
TechnikMax opened this issue Nov 11, 2023 · 10 comments · Fixed by #2777
Closed

Old logo for X (formally Twitter) #2775

TechnikMax opened this issue Nov 11, 2023 · 10 comments · Fixed by #2777

Comments

@TechnikMax
Copy link

TechnikMax commented Nov 11, 2023

Describe the bug
X / Twitter an a new logo and i didn't saw any option to use it.

To Reproduce
Steps to reproduce the behavior:
Use the twitter option from the social package

Expected behavior
The new X-Logo

MJML environment

  • MJML Version: 4.14.1

Additional context
I see two solutions, but i can't fix them myself because bot need a new logo on the mailjet server

  1. Just replace the image as this is the new one.
  2. Create a new social network "X" to choose

Personally I like the second option better, firstly nothing changes for the user, secondly the name is correct and not mixed up with the old name and the new logo.

@npracht
Copy link
Contributor

npracht commented Nov 21, 2023

I think we could add here :

  x: {
    'share-url': 'https://twitter.com/intent/tweet?url=[[URL]]',
    'background-color': '#55acee',
    src: `${IMG_BASE_URL}x.png`,
  },

And we also need Mailjet to add the x.png file on there server: https://www.mailjet.com/images/theme/v1/icons/ico-social/

@npracht
Copy link
Contributor

npracht commented Nov 21, 2023

I sent an email to mailjet product team to ask them to add the logo. Then I'll do the PR.

@npracht
Copy link
Contributor

npracht commented Nov 21, 2023

I created the PR, we need to wait Mailjet to add the icon : #2776

@npracht
Copy link
Contributor

npracht commented Nov 21, 2023

@ngarnier who is project maintainer now ?

@iRyusa
Copy link
Member

iRyusa commented Nov 21, 2023

As stated in multiple issues, we won't update the list of default social network just use regular social-element to host your own icon instead.

@iRyusa iRyusa closed this as not planned Won't fix, can't repro, duplicate, stale Nov 21, 2023
@npracht
Copy link
Contributor

npracht commented Nov 21, 2023

Maybe you can explain why ? It is easy and one of the main social networks worldwide.

@iRyusa
Copy link
Member

iRyusa commented Nov 21, 2023

When MJML3 was out custom social element was really painful to write.
MJML4 added mj-social-element to ease custom social icons, removing the need of maintaining a list of social networks. Multiple times, people ask to add every new or local social network and would require a new version of MJML everytime : that's not something I want to do.

Last time Mailjet simply did remove old logos #2482 #2547 #2551 opening multiple issues, reaching me personally on Slack/Twitter/Mail.

<mj-social-element src="path-to-icon" href="my-profile" /> or <mj-social-element name="x" href="my-profile" /> at least you know that you have to handle hosting your icons instead of relying on something you can't control.

@npracht
Copy link
Contributor

npracht commented Nov 21, 2023

I understand what you say, but then we should remove all native icons from repo to avoid maintenance ?
Otherwise, I don't see how you can refuse maintenance of an existing social network, while all is ready, PR is offered by community member.

@npracht
Copy link
Contributor

npracht commented Nov 21, 2023

@TechnikMax here is the PR you need : #2777
I hope @iRyusa changes his mind, refusing the contribution in this context is a non sense from an Open Source government perspective.

Otherwise, you can simply cherrypick the PR into your project.

@iRyusa
Copy link
Member

iRyusa commented Nov 21, 2023

Those were kept for MJML3 compatibility (smooth transition with mj-migrate) and should be marked as deprecated you're right. That was a big discussion when moving from MJML 3 to 4 and we should have stated this more clearly.

I'll update the doc to reflect this, and I probably won't merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants