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

Replace xitter links with Bluesky (#15988) #15989

Merged
merged 6 commits into from
Feb 11, 2025

Conversation

maureenlholland
Copy link
Collaborator

@maureenlholland maureenlholland commented Feb 11, 2025

If this changeset needs to go into the FXC codebase, please add the WMO and FXC label.

One-line summary

Replaces xitter links with bluesky links and adds bluesky icons

Significant changes and points to review

Issue / Bugzilla link

#15988

Testing

Screenshot 2025-02-11 at 1 59 13 PM Screenshot 2025-02-11 at 1 32 13 PM Screenshot 2025-02-11 at 1 40 44 PM Screenshot 2025-02-11 at 1 49 40 PM Screenshot 2025-02-11 at 1 37 05 PM Screenshot 2025-02-11 at 3 12 35 PM

@maureenlholland maureenlholland requested a review from a team as a code owner February 11, 2025 18:52
@maureenlholland maureenlholland added the WMO and FXC Code relevant to both the WMO and FXC projects label Feb 11, 2025
@maureenlholland maureenlholland linked an issue Feb 11, 2025 that may be closed by this pull request
Copy link
Contributor

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

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

Few questions for consistency:

@@ -66,7 +66,7 @@ <h2 class="moz24-footer-label" data-testid="footer-heading-developers">
<div class="moz24-footer-refresh-social-wrapper">
<h2 class="moz24-footer-heading-social">{{ ftl('footer-refresh-follow-mozilla') }}</h2>
<ul class="moz24-footer-links-social">
<li><a class="twitter" href="{{ mozilla_twitter_url() }}" data-link-position="footer" data-link-text="Twitter (@mozilla)">{{ ftl('footer-refresh-x-formerly-twitter', fallback='footer-twitter') }}<span> (@mozilla)</span></a></li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can both the helpers

def mozilla_twitter_url(ctx):
"""Output a link to Twitter taking locales into account.
Uses the locale from the current request. Checks to see if we have
a Twitter account that match this locale, returns the localized account
url or falls back to the US account url if not.
be removed now, incl. tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I was checking if we wanted to repurpose for Bluesky, but will remove as it seems we don't have localised account there yet

@@ -8,8 +8,7 @@ footer-refresh-advertise = Advertise with { -brand-name-mozilla }
footer-refresh-firefox-release-notes = { -brand-name-firefox } Release Notes
footer-refresh-mdn = MDN
footer-refresh-follow-mozilla = Follow @{ -brand-name-mozilla }
footer-refresh-x = X
footer-refresh-x-formerly-twitter = X (formerly Twitter)
footer-refresh-bluesky = Bluesky
Copy link
Contributor

Choose a reason for hiding this comment

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

Sometimes it's here "Bluesky" literally, sometimes {-brand-name-bluesky} reference …

IIUC third party brands should no longer be added to the brand terms, so probably can be used spelled out fully in every single string(?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will check

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sometimes it's here "Bluesky" literally, sometimes {-brand-name-bluesky} reference

this was a miss on my part, I meant to use the brand name ref consistently, still checking on the brand term standards

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed internally, we're not going to include the 3rd party brand in FTL files (unless it's part of a larger sentence, in which case we should use a comment to indicate the words that should not be translated), will put directly in HTML

@maureenlholland maureenlholland added the WIP 🚧 Pull request is still work in progress label Feb 11, 2025
@maureenlholland maureenlholland requested a review from a team as a code owner February 11, 2025 19:54
@maureenlholland maureenlholland added Review: XS Code review time: up to 30min Frontend HTML, CSS, JS... client side stuff Needs Review Awaiting code review and removed WIP 🚧 Pull request is still work in progress labels Feb 11, 2025
@maureenlholland maureenlholland changed the title Replace xitter links with Bluesky Replace xitter links with Bluesky (#15988) Feb 11, 2025
@wen-2018 wen-2018 self-assigned this Feb 11, 2025
@wen-2018 wen-2018 removed the Needs Review Awaiting code review label Feb 11, 2025
Copy link
Collaborator

@wen-2018 wen-2018 left a comment

Choose a reason for hiding this comment

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

Question: I see some Twitter links for the Nightly templates, ex: /Users/jingwen/Documents/projects/bedrock/bedrock/firefox/templates/firefox/nightly/firstrun.it.html <a href="https://twitter.com/mozilla_fr/">@mozilla_fr sur Twitter</a> , should they be removed too?

@maureenlholland
Copy link
Collaborator Author

@wen-2018 it's a good question. I've asked internally. I think they should be removed eventually but question whether it should be as part of this PR or a separate one to follow up on the less visible links (and share urls).

Copy link
Collaborator

@wen-2018 wen-2018 left a comment

Choose a reason for hiding this comment

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

The current changes LGTM! r+
Per the internal discussions, other clean ups can be in a separate PR

@wen-2018 wen-2018 merged commit 3a96be0 into mozilla:main Feb 11, 2025
6 checks passed
@alexgibson alexgibson added Applied to Springfield Has been applied to Springfield after having WMO + FXC label and removed WMO and FXC Code relevant to both the WMO and FXC projects labels Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Applied to Springfield Has been applied to Springfield after having WMO + FXC label Frontend HTML, CSS, JS... client side stuff Review: XS Code review time: up to 30min
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants