Fixed create report fields flow#58521
Conversation
|
@hungvu193 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] |
|
@paultsimura PR is ready for review |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb Chromechrome.webmiOS: NativeSimulator.Screen.Recording.-.iPhone.15.Pro.-.2025-03-15.at.19.27.24.mp4iOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.15.Pro.-.2025-03-15.at.19.47.30.mp4MacOS: Chrome / SafariScreen.Recording.2025-03-15.at.19.20.20.movMacOS: DesktopScreen.Recording.2025-03-15.at.21.59.01.mov |
There was a problem hiding this comment.
We've agreed that we need to add a comment explaining why we need to check if the string is defined even though the type of formInitialValue is non-nullable string.
Please add it here and I'll approve the PR.
| } | ||
|
|
||
| if (type === CONST.REPORT_FIELD_TYPES.TEXT && formInitialValue.length > CONST.WORKSPACE_REPORT_FIELD_POLICY_MAX_LENGTH) { | ||
| // As formInitialValue can be undefined and ts can't identify this yet, we are using !!formInitialValue to check for that. |
There was a problem hiding this comment.
This comment doesn't explain why formInitialValue can be undefined.
Please add a note that it happens because the InitialValue input is rendered conditionally, and if it's not been rendered when the validation is executed, formInitialValue will be undefined.
There was a problem hiding this comment.
Can you make a suggestion for this comment? I will update this accordingly.
There was a problem hiding this comment.
You can take it from my comment above.
There was a problem hiding this comment.
@paultsimura I am sorry, but can you write a suggestion in GH so that I can amend that directly?
There was a problem hiding this comment.
I cannot at the moment.
Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>
|
✋ 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/mountiny in version: 9.1.15-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.1.15-9 🚀
|
Explanation of Change
Fixed Issues
$ #58135
Tests
Offline tests
Same as Tests
QA Steps
Same as Tests
PR 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
Screen.Recording.2025-03-15.at.1.10.36.AM.mov