-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Translate alternate text to Spanish #7552
Translate alternate text to Spanish #7552
Conversation
src/libs/OptionsListUtils.js
Outdated
? (hasMultipleParticipants && lastActorDetails | ||
? `${lastActorDetails.displayName}: ` | ||
: '') | ||
+ Str.htmlDecode(report.lastMessageText) | ||
+ (isLastMessageAttachment ? `[${Localize.translateLocal('common.attachment')}]` : Str.htmlDecode(report.lastMessageText)) |
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 we please simplify this ternary? lets not nest it multi levels.
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.
This should be okay. Any suggestion?
const lastMessageTextFromReport = isLastMessageAttachment ? `[${Localize.translateLocal('common.attachment')}]` : Str.htmlDecode(report.lastMessageText);
const lastMessageText = report
? (hasMultipleParticipants && lastActorDetails
? `${lastActorDetails.displayName}: `
: '')
+ (lastMessageTextFromReport)
: '';
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.
Let's break the concatenation into the next line.
const lastMessageTextFromReport = isLastMessageAttachment ? `[${Localize.translateLocal('common.attachment')}]` : Str.htmlDecode(report.lastMessageText);
let lastMessageText = report && hasMultipleParticipants && lastActorDetails
? `${lastActorDetails.displayName}: `
: '';
lastMessageText += report? lastMessageTextFromReport : '';
src/libs/OptionsListUtils.js
Outdated
@@ -221,17 +221,17 @@ function createOption(personalDetailList, report, { | |||
const personalDetail = personalDetailList[0]; | |||
const hasDraftComment = hasReportDraftComment(report); | |||
const hasOutstandingIOU = lodashGet(report, 'hasOutstandingIOU', false); | |||
const isLastMessageAttachment = report ? ReportUtils.isReportMessageAttachment(report.lastMessageText) : false; |
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.
const isLastMessageAttachment = report ? ReportUtils.isReportMessageAttachment(report.lastMessageText) : false; |
src/libs/OptionsListUtils.js
Outdated
? `${lastActorDetails.displayName}: ` | ||
: '') | ||
+ Str.htmlDecode(report.lastMessageText) | ||
const lastMessageTextFromReport = report && (isLastMessageAttachment ? `[${Localize.translateLocal('common.attachment')}]` : Str.htmlDecode(report.lastMessageText)); |
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.
const lastMessageTextFromReport = report && (isLastMessageAttachment ? `[${Localize.translateLocal('common.attachment')}]` : Str.htmlDecode(report.lastMessageText)); | |
const lastMessageTextFromReport = ReportUtils.isReportMessageAttachment(lodashGet(report, 'lastMessageText','')) ? `[${Localize.translateLocal('common.attachment')}]` : Str.htmlDecode(lodashGet(report, 'lastMessageText','')); |
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.
LGTM.
cc: @roryabraham
🎀 👀 🎀 C+ reviewed
@sobitneupane Please tick all the platforms on the template and attach screenshots for all. |
okay |
@sobitneupane I have one small tip for you to keep in mind for the future. When adding screenshots in the pull request description, GitHub has a tendency to make the images way too big, especially when pasting a screenshot of a mobile device/simulator. I find it very helpful to adjust those images to a more reasonable size by either:
This is helpful not only to reviewers, but also to our QA team. Thanks! |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Thanks for the suggestion. Will definitely take care moving forward. |
🚀 Deployed to staging by @roryabraham in version: 1.1.39-0 🚀
|
Issue 1 - Title- [Medium]: Voiceover +Safari: Screen reader : Screen reader is not reading the Role, State & Value for the 'Language' control. 7552_Screen.reader.is.not.reading.the.Role.State.Value.for.the.Language.control.MOVIssue 2 - Title- [Medium]:Chrome +Jaws: Screen reader : Screen reader is not reading the Label along with the value. 7552_Label.is.not.associated.with.the.value.of.the.combo.box.mp4Issue 3 - Title- [Medium]: Chrome +Talkback: Screen reader : Focus stuck on the 'Language' control twice. 7552_Stuck.twice.mp4Issue 4 - Title- [Medium]: Talkback+ Chrome: Screen reader : Incorrect role and state is defined for the 'Language' control. 7552_Incorrect.state.mp4Issue 5 - Title- [Medium]: Talkback+ Chrome: Screen reader :'Language' control drop down is not accessible using screen reader. 7552_2.1.1_.Language.control.drop.down.is.not.accessible.using.screen.reader.mp4 |
Details
[Attachment] alternate text in LHN is not translated to Spanish
Fixed Issues
$ #7440
Tests
QA Steps
Tested On
Screenshots
Web
attachment-translation.mp4
Mobile Web
Desktop
iOS
Android