task(settings): add fallback text for linked account 3rd-party auth#19450
task(settings): add fallback text for linked account 3rd-party auth#19450
Conversation
| deeplink?: string; | ||
| flowQueryParams?: QueryParams; | ||
| separatorTextId?: string; | ||
| separatorText?: string; |
There was a problem hiding this comment.
Sorry to be pushing back so much heh😅
We've kind of conventionally called this something like, localizedSeparatorText to indicate the text being passed in does not need localization (search the codebase for localizedHeading, localizedDescription, localizedText, localizedCTAText, and there are others). I think we should either have every instance of this component have that localized text passed into it (using ftlMsgResolver and not l10n because that enforces fallback text is provided and we hope to turn l10n tests on for one of these days which also would have caught the missing ID!), or we can pass in a prop like separatorType instead which can default to "or". Then, you can conditionally render the correct <FtlMsg> based on separatorType. Dealer's choice!
Also... our #9E9E9E text-grey-300 does not pass AA ADA color contrast even at "normal" text, so at "extra light", it most certainly should be darkened.
There was a problem hiding this comment.
@LZoog see latest! Does that work? I didn't test text-grey-600 because my local is still broken 😮💨 Happy to change if needed!
There was a problem hiding this comment.
Per discussion AA standard request completed via #19485
toufali
force-pushed
the
add-fallback-separator-text
branch
2 times, most recently
from
2a16c36 to
3e0813a
Compare
LZoog
left a comment
LZoog
left a comment
There was a problem hiding this comment.
LGTM, thank you for sticking with this!!
Mentioned this in Slack but bonus points also if we get a new storybook state added for ThirdPartyAuth, that's a non-blocking request though 🙂
toufali
force-pushed
the
add-fallback-separator-text
branch
from
3e0813a to
d5930f0
Compare
2 participants
Because
This pull request
Issue that this pull request solves
Closes: FXA-12397
Other information (Optional)
In response to #19441 (comment)