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

Share widget: updating Twitter image and name #9734

Merged
merged 1 commit into from
May 15, 2024

Conversation

Garneauma
Copy link
Contributor

Updating Twitter image to the "X" icon.
Updating the "Twitter" name to "X".

Keeping the JS property as "twitter" so it doesn't trigger a major release.

@duboisp
Copy link
Member

duboisp commented Feb 19, 2024

Pre-approved upon review and testing

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Can you add the filtering support for "x" as an alternative for "twitter"?

This change is a minor change evaluated as follow:

  • Minor perceptible change: Twitter text and logo are switching to "X"
  • Minor configuration change: Add options to filter by specifying a new option named "X"
  • Patch assets change: Update of the asset /wet-boew/assets/sprites_share.png

src/plugins/share/share.js Show resolved Hide resolved
@Garneauma Garneauma added this to the v4.0.80 milestone May 13, 2024
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

See the suggestion.

An example when both "X" and "Twitter" are defined was missing.

I tested everything and it does work as expected. It will be good to merge once the requested content change are completed and validated.

Note: This change are going to impact the CID scripts for GCWeb because it is a dependency copied over manually, see for your information:

Note2: This change are going to update the sprite assets into our distribution package.

src/plugins/share/share-en.hbs Outdated Show resolved Hide resolved
src/plugins/share/share-en.hbs Show resolved Hide resolved
src/plugins/share/share-en.hbs Show resolved Hide resolved
src/plugins/share/share-fr.hbs Outdated Show resolved Hide resolved
src/plugins/share/share-fr.hbs Show resolved Hide resolved
src/plugins/share/share-fr.hbs Show resolved Hide resolved
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Reviewed and tested locally, everything is demoed and work as expected.

This is a minor change because it add the "X social media" option
And the switch from Twitter to X is also a Minor change only because it is visually perceptible.

@duboisp duboisp merged commit 4df2b3e into wet-boew:master May 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review in progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants