-
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
Saif expapptest #53750
Saif expapptest #53750
Conversation
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
@hoangzinh 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] NOTE: It looks like |
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Hi, I am trying to sign the Contributor License Agreement (CLA) for my contribution, but the link to the CLA.md file appears to be broken (404 - Page Not Found). Could you please provide the correct link or guide me on how I can sign the CLA? Thank you! |
@cla-assistant recheck, I have signed the CLA as per the instructions in the document. |
For CLA, just follow this comment #53750 (comment) |
I have read and signed the CLA. |
I have read and signed the CLA. what next @hoangzinh |
Please hold on to any further actions on this PR, because you haven't hired yet so you should not spend effort on this PR. #53547 (comment) |
Was the contributor ever hired? If not, can we close this PR? Thanks! |
@shawnborton Nope he wasn't. I'm unable to close this PR. @saifelance can you close this PR? We can always re-open this PR if you're hired later. |
I have spent a significant amount of time explaining the frontend and backend issues, but I feel my efforts and time have not been respected. |
We always appreciate your efforts here @saifelance. However, we have to follow with Expensify Contributing Guides. Please ensure you have read it, especially this section https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#working-on-expensify-jobs |
Explanation of Change
Fixed Issues
$ #53547
PROPOSAL: #53547 (comment)
Tests
Offline tests
QA Steps
Same as tests
Verify that no errors appear in the JS console
Offline tests
QA Steps
Key Changes:
1. Cached IDs:
Added useState to store the last valid
parentReportID
andparentReportActionID
.useEffect updates these states whenever valid values are provided.
2. Fallback Logic:
Use cached values (
cachedParentReportID
,cachedParentReportActionID
) if the backend returns null or empty values.3. No Breakage on
Null
IDs:Prevents navigation from breaking by ensuring fallback values are always available.
Consider Adding Logs for Debugging:
Add logs to identify when cached values are being used:
console.log('Using cached ID:', {finalParentReportID, finalParentReportActionID});
// TODO: These must be filled out, or the issue title must include "[No QA]."PR Author Checklist
Contributor License Agreement (CLA)
I hereby sign the CLA as a contributor to the Expensify project. I agree to the terms and conditions outlined in the CLA document.
Reviewer Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)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
392903263-30cfb07b-a0a3-438b-9852-d0aa5ea509c0.mov
Android: mWeb Chrome
392903263-30cfb07b-a0a3-438b-9852-d0aa5ea509c0.mov
iOS: Native
392903263-30cfb07b-a0a3-438b-9852-d0aa5ea509c0.mov
iOS: mWeb Safari
392903263-30cfb07b-a0a3-438b-9852-d0aa5ea509c0.mov
MacOS: Chrome / Safari
392903263-30cfb07b-a0a3-438b-9852-d0aa5ea509c0.mov
MacOS: Desktop
392903263-30cfb07b-a0a3-438b-9852-d0aa5ea509c0.mov