-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
fix: Chat - Most of the workspace modifying settings messages are not translated in #Admins. #53238
fix: Chat - Most of the workspace modifying settings messages are not translated in #Admins. #53238
Conversation
… translated in #Admins. Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
…OG_SET_CATEGORY_NAME'. Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
…LF_APPROVAL change logs. Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
…BILLABLE. Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
@rushatgabhane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@rushatgabhane, the jest unit test fail is not related to our PR, I have merged main to see if that fixes it. BTW, do you have any idea how to write test for this PR? Do we have to write test for each translation? |
@rushatgabhane friendly bump. |
Signed-off-by: krishna2323 <[email protected]>
@Krishna2323 could you please add test steps and screenshots |
@rushatgabhane, I'm not sure what to add in the test steps and recordings because it will take great amount of time if we try to add test step and recording for each translation. @techievivek any suggestions? |
@Krishna2323 I think we can randomly test for 1-2 translations, that should be enough here. |
Signed-off-by: krishna2323 <[email protected]>
Signed-off-by: krishna2323 <[email protected]>
@rushatgabhane, tests steps and recordings are updated. |
/** Old role of user */ | ||
oldValue?: string; | ||
/** Old role of user or old value of the category/tag field */ | ||
oldValue?: boolean | string; |
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.
@Krishna2323 why can this be boolean? Maybe you can give an example where it'll be boolean, and string.
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.
Updated to:
/** Old role of user or old value (enabled/disable) of the category/tag field.
* The user role will be of type string and category/tag value (enabled/disable)
* will be of type boolean.
*/
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.
interesting..
why don't we have a new key for this? As a user of this field, I will have to type check every time and might not know how to handle it.
Signed-off-by: krishna2323 <[email protected]>
Reviewer Checklist
Screenshots/VideosAndroid: mWeb Chrome |
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
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.
Looks good, great work.
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/techievivek in version: 9.0.99-0 🚀
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.0.99-2 🚀
|
Explanation of Change
Fixed Issues
$ #52984
PROPOSAL: #52984 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
android_native.mp4
Android: mWeb Chrome
android_chrome.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4
MacOS: Desktop
desktop_app.mp4