fix: Workspace - The new WS names appear in English when Spanish is set up.#55850
fix: Workspace - The new WS names appear in English when Spanish is set up.#55850MonilBhavsar merged 28 commits intoExpensify:mainfrom
Conversation
…et up. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@eVoloshchak 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] |
|
@eVoloshchak, I haven't uploaded the recording for android native cause I'm facing app crash as soon as it starts. I will try to fix that and update the recordings if possible. |
src/languages/params.ts
Outdated
|
|
||
| type GoToRoomParams = {roomName: string}; | ||
|
|
||
| type NewWorksacepNameParams = {userName: string; workspaceNumber?: number}; |
There was a problem hiding this comment.
| type NewWorksacepNameParams = {userName: string; workspaceNumber?: number}; | |
| type NewWorksaceNameParams = {userName: string; workspaceNumber?: number}; |
src/libs/actions/Policy/Policy.ts
Outdated
| .map((policy) => workspaceRegex.exec(policy?.name ?? '')) | ||
| .filter(Boolean) // Remove null matches | ||
| .map((match) => Number(match?.[1] ?? '0')); | ||
| const lastWorkspaceNumber = workspaceNumbers.length > 0 ? Math.max(...workspaceNumbers) : -Infinity; |
There was a problem hiding this comment.
Can we use null or undefined instead of -Infinity?
There was a problem hiding this comment.
updated to undefined
src/libs/actions/Policy/Policy.ts
Outdated
| .map((match) => Number(match?.[1] ?? '0')); | ||
| const lastWorkspaceNumber = workspaceNumbers.length > 0 ? Math.max(...workspaceNumbers) : -Infinity; | ||
|
|
||
| return lastWorkspaceNumber !== -Infinity |
There was a problem hiding this comment.
| return lastWorkspaceNumber !== -Infinity | |
| return translateLocal('workspace.new.workspaceName', {userName: displayNameForWorkspace, workspaceNumber: lastWorkspaceNumber ? lastWorkspaceNumber + 1 : undefined}) |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@eVoloshchak friendly bump. |
src/libs/actions/Policy/Policy.ts
Outdated
| const lastWorkspaceNumber = Math.max(...parsedWorkspaceNumbers); | ||
| return lastWorkspaceNumber !== -Infinity ? `${defaultWorkspaceName} ${lastWorkspaceNumber + 1}` : defaultWorkspaceName; | ||
| const escapedName = escapeRegExp(displayNameForWorkspace); | ||
| const workspaceRegex = new RegExp(`(?:${escapedName}'s Workspace|Espacio de trabajo)\\s*(\\d+)?\\s*(?:de ${escapedName})?$`, 'i'); |
There was a problem hiding this comment.
This will partially stop working when we add a third language. Could we instead iterate through locale files in src/languages and look for workspaceName values there?
There was a problem hiding this comment.
I updated this to:
const workspaceTranslations = CONST.LANGUAGES.map((lang) => translate(lang, 'workspace.common.workspace')).join('|');
const workspaceRegex = new RegExp(`(?:${escapedName}'s ${workspaceTranslations})\\s*(\\d+)?\\s*(?:de ${escapedName})?$`, 'i');|
And could you please take a look at GH checks failing? Merging with main should resolve some of it |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@eVoloshchak everything resolved. |
|
@eVoloshchak friendly bump. |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-03-03.at.15.58.23.movAndroid: mWeb Chromescreen-20250303-160106.mp4iOS: NativeScreen.Recording.2025-03-03.at.15.43.36.moviOS: mWeb SafariScreen.Recording.2025-03-03.at.15.45.15.movMacOS: Chrome / SafariScreen.Recording.2025-03-03.at.15.38.30.movMacOS: DesktopScreen.Recording.2025-03-03.at.15.35.02.mov |
|
@Krishna2323, screen recording for Android: Native is missing from the checklist. Could you add it please? |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Always good to add a test, if we can |
|
Test will be added today, @eVoloshchak could you please check this comment? Thanks |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@eVoloshchak unit test is added. |
| const lastWorkspaceNumber = Math.max(...parsedWorkspaceNumbers); | ||
| return lastWorkspaceNumber !== -Infinity ? `${defaultWorkspaceName} ${lastWorkspaceNumber + 1}` : defaultWorkspaceName; | ||
| const escapedName = escapeRegExp(displayNameForWorkspace); | ||
| const workspaceTranslations = CONST.LANGUAGES.map((lang) => translate(lang, 'workspace.common.workspace')).join('|'); |
| import Onyx from 'react-native-onyx'; | ||
| import {translateLocal} from '@libs/Localize'; | ||
| import BaseLocaleListener from '@libs/Localize/LocaleListener/BaseLocaleListener'; | ||
| // eslint-disable-next-line no-restricted-syntax |
There was a problem hiding this comment.
| // eslint-disable-next-line no-restricted-syntax |
Will deleting this line result in a linter error?
|
Also, could you add the screen recording for Android: Native, please? (it looks like it wasn't uploaded fully) |
eVoloshchak
left a comment
There was a problem hiding this comment.
@Krishna2323, this stopped incrementing the workspace name (it was working before, there were no changes besides merging with main, weird)
Screen.Recording.2025-02-27.at.18.46.22.mov
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
| const workspaceNumbers = Object.values(allPolicies) | ||
| .map((policy) => workspaceRegex.exec(policy?.name ?? '')) | ||
| .filter(Boolean) // Remove null matches | ||
| .map((match) => Number(match?.[1] ?? '0')); |
There was a problem hiding this comment.
@Krishna2323, this stopped incrementing the workspace name (it was working before, there were no changes besides merging with main, weird)
@eVoloshchak, I mistakenly removed the default value "0" when mapping over workspace number and that resulted in NaN which caused the issue. Added the default value back.
There was a problem hiding this comment.
Works well now, thanks! Could you resolve the conflicts please?
There was a problem hiding this comment.
conflicts resolved.
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
9d1e75a to
b1f617c
Compare
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@Krishna2323, could you change the last step to this please?
|
MonilBhavsar
left a comment
There was a problem hiding this comment.
Looks good! Thanks for staying on this
|
✋ 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/MonilBhavsar in version: 9.1.9-0 🚀
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.1.9-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.1.9-8 🚀
|
Explanation of Change
Fixed Issues
$ #53599
PROPOSAL: #53599 (comment)
Tests
New Workspace{userName}'s Workspace> ConfirmNew Workspace{userName}'s Workspace 1Offline tests
New Workspace{userName}'s Workspace> ConfirmNew Workspace{userName}'s Workspace 1QA Steps
New Workspace{userName}'s Workspace> ConfirmNew Workspace{userName}'s Workspace 1PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.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