Skip to content

fix(settings): missing 3rd-party auth string#19441

Merged
toufali merged 1 commit intomainfrom
fix-missing-3rdpartyauth-string
Sep 11, 2025
Merged

fix(settings): missing 3rd-party auth string#19441
toufali merged 1 commit intomainfrom
fix-missing-3rdpartyauth-string

Conversation

@toufali
Copy link
Copy Markdown
Member

@toufali toufali commented Sep 11, 2025

Because

  • new string was overwritten by PR changes/rebase

This pull request

  • reintroduces the string

@toufali toufali requested a review from a team as a code owner September 11, 2025 21:19

# For the sign-in page, when 3rd-party auth is the only option, this string appears with a divider line between the user's avatar on top and 3rd-party authentication buttons (continue-with-google continue-with-apple buttons) on bottom.
# This could also be translated as "Sign in with the following" or "Sign in with the below".
third-party-auth-options-sign-in-with = Sign in with
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I might be missing something but where is the fall back text for this? If we want to go with it just being "or", I'd just call out this is the only place where we'd rely on Fluent to replace the text for us 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@LZoog It defaults to "or" here:

<FtlMsg id={separatorTextId || 'third-party-auth-options-or'}>
– is there a better place to hold the default, or would you like me to add context about the default in the l10n comment?

Copy link
Copy Markdown
Contributor

@LZoog LZoog Sep 11, 2025

Choose a reason for hiding this comment

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

It doesn't matter to translators, more of an us thing so if Fluent doesn't replace the text or the ID is missing or any other reason, the correct English string would display. It would be my preference that "or" is used as the default for the corresponding FTL ID, and "Sign in with" has its own <FtlMsg> wrapper. I think this would need some tweaking so they've got their own FtlMsg:

<FtlMsg id={separatorTextId || 'third-party-auth-options-or'}>
  <div className="mx-4 text-base text-grey-300 font-extralight">
    or
  </div>
</FtlMsg>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can be done in another PR if you want btw, I know this one's just to add the string back, I'll r+ to unblock Bryan in case he's trying to get these over to Pontoon.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The better way is to pass in the localized string directly instead of passing in the FTL id, so that you can pass in ftlMessageResolver.getMsg('custom-ftl-id', 'Customized string') and get the customized fallback.

Copy link
Copy Markdown
Contributor

@LZoog LZoog Sep 11, 2025

Choose a reason for hiding this comment

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

Yep, that works too and agreed is preferable in this case!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

will merge this for translation and improve under separate PR 👍

@toufali toufali merged commit 234a1d8 into main Sep 11, 2025
19 checks passed
@toufali toufali deleted the fix-missing-3rdpartyauth-string branch September 11, 2025 22:14
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.

4 participants