-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[Sheets] [Pt. 1] Root PR #5557
[Sheets] [Pt. 1] Root PR #5557
Conversation
|
Co-authored-by: Samuel Newman <mozzius@protonmail.com>
importantForAccessibility
Co-authored-by: Eric Bailey <git@esb.lol> Co-authored-by: Samuel Newman <mozzius@protonmail.com>
Co-authored-by: Eric Bailey <git@esb.lol> Co-authored-by: Samuel Newman <mozzius@protonmail.com>
Co-authored-by: Eric Bailey <git@esb.lol> Co-authored-by: Samuel Newman <mozzius@protonmail.com>
Co-authored-by: Eric Bailey <git@esb.lol> Co-authored-by: Samuel Newman <mozzius@protonmail.com>
Co-authored-by: Eric Bailey <git@esb.lol> Co-authored-by: Samuel Newman <mozzius@protonmail.com>
Co-authored-by: Eric Bailey <git@esb.lol> Co-authored-by: Samuel Newman <mozzius@protonmail.com> Co-authored-by: dan <dan.abramov@gmail.com>
Co-authored-by: Samuel Newman <mozzius@protonmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goated fr ✨
<Trans>Appeal "{strings.name}" label</Trans> | ||
</Text> | ||
<Text style={[a.text_md, a.leading_snug]}> | ||
<Trans>This appeal will be sent to</Trans>{' '} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is inappropriate to leave the labeler name outside the <Trans>
tag. This prevents proper localization in languages with different word order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea this was accidentally left behind, should have been reverted. Merged in changes to fix this, thanks for noticing and commenting about it!
) : ( | ||
<> | ||
<Text style={[t.atoms.text, a.text_md, a.leading_snug, a.mt_lg]}> | ||
<Trans>This label was applied by </Trans> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue here.
<Trans> | ||
This label was applied by{' '} | ||
<InlineLinkText | ||
label={desc.source || _(msg`an unknown labeler`)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And could you please provide the full sentence when an unknown labeler
is displayed so that it can be localized?
As when applied by the author above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you verify (either by replying here or in the linked PR) that this fixes the problem here? https://github.com/bluesky-social/social-app/pull/5616/files#diff-70b5ca5a14b0410abcadacda7727faa8219f009b561533160ee75331fdd545da
(@tkusano thank you!)
This is a base PR for each stacked Sheets PR. Instead of merging each into main, start from #5560 and merge them down into this PR. Once everything is reviewed and ready to go, this PR can get merged into main.
This PR does not add any new behavior, just removes a bit of code so not much testing is needed on this one. Check each PR for a test plan.