Repository navigation
Conversation
Clear the linked tab on the route of the Connections screen itself, so it cannot rewrite another route while Begin is still navigating (Expensify#105713). Bring back the "Chat with your account executive" link that the old Accounting page showed (Expensify#105711). Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
The badge and button leave little room for the status message on the Connections card, so show it with TextWithTooltip (Expensify#105722). Signed-off-by: krishna2323 <belivethatkg@gmail.com>
A plain goBack from the Gusto panel after a sync landed on a full-page Not here (Expensify#105690). Go back to the Connections route explicitly, as the HR, Accounting and Receipt partners panels did before the launch. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
The connect-flow params were cleared with the global setParams, which can change another route while a navigation is still running, the same as the tab param in Expensify#105713. Clear them on the Connections screen route instead. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
The HR, Recruiting and Accounting panels go back to Connections, which is wrong when Members, Workflows, Rules or Home opened them directly. Point those links at the matching Connections tab, as they led to a full workspace page before the launch. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…help link Signed-off-by: krishna2323 <belivethatkg@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64a6f7f95d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari105713.mov105713.mp4105722.mov105735.mov |
|
#105690 was fixed too? |
dylanexpensify
left a comment
dylanexpensify
left a comment
There was a problem hiding this comment.
Good from product POV
The results waited for MODAL.isVisible to clear, which never happens while an RHP is open, so they only opened after the panel closed. Wait only for a modal opening on top, and let only a focused screen track a sync so Connections under the panel does not open them again (Expensify#105690). Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…nfirms it Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@MelvinBot review |
ReviewThe fixes look correct for the three linked blockers. I found one plausible regression in the sync-results hook and one small accessibility loss. CI is green apart from the approval check. 1. Results can open twice when a sync starts before the panel opens (plausible)
Before this PR, step 3 didn't happen because the panel never opened the results. Consider clearing 2. Screen readers no longer read the status message (minor)
3. Nit: The PR description says "Fixes four deploy blockers", but it lists three. Checked and fine
|
…card label A screen covered partway through a sync now lets the screen on top show its results instead of opening them again on refocus. The Connections card label includes the status message again, now that it renders as an addon. Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@MelvinBot Thanks, both fixed.
|
|
Thanks. I checked 2869ecaa, and all three points are resolved:
One trade-off, not a blocker: a screen covered by something other than a sync-results listener now skips the results. For example, Members with a member's details panel open would skip them. That seems fine, because the results belong to whatever is showing when the sync finishes. Jest and perf tests were still running when I checked. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@situchan any updates on this? |
|
How were you able to test #105690? Any test credentials for Gusto? |
I created a new account for testing. |
lakchote
changed the title
lakchote
left a comment
lakchote
left a comment
There was a problem hiding this comment.
Thanks @Krishna2323, looks good. NAB: it looks like if you open another page from the HR panel while a sync runs, the results won't open when you go back, since both covered screens drop them now. Can you check that in a follow-up?
|
🚧 lakchote has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
…ns-launch-regressions [CP Staging] fix: restore Connections tab param scoping and the accounting help link (cherry picked from commit ab3ccbd) (cherry-picked to staging by lakchote)
|
🚀 Cherry-picked to staging by https://github.com/lakchote in version: 9.5.28-6 🚀
|
|
No help site update needed: this PR fixes three deploy-blocker bugs (wrong landing tab, a missing tooltip on cut-off status text, and Back buttons that led to Not here), and no existing article sentence becomes incorrect. |
|
🚀 Cherry-picked to staging by https://github.com/lakchote in version: 9.5.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.5.30-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Fixes three deploy blockers caused by #105137.
#105713: Begin on "Connect to QuickBooks Online" opens the Workspaces tab (HybridApp)
Cause:
connections?tab=accounting.tabwith the globalNavigation.setParams, which changes whichever route is focused at that moment.Fix:
tabwith the screen's ownnavigation.setParams, which only changes the Connections route.newConnectionNameand the others an upgrade or setup flow returns with), which had the same global call.#105722: No tooltip on a cut-off connection status
Cause: the card shows the status message on one line next to the badge and the Fix or Configure button. Long messages like "Cannot sync due to broken connection" get cut off with no way to read them.
Fix:
TextWithTooltip, which shows the full text on hover only when it's cut off. Native renders plain text, so mobile is unchanged.MenuItem'sdescription.#105690: Not here after syncing Gusto and pressing Back
Cause:
MODAL.isVisibleto clear before opening the results.BaseModalkeeps that flag on for as long as an RHP is open, so the panel queued the results until it closed./hr-sync-results/hr-sync-results), which is Not here.goBack().Fix:
willAlertModalBecomeVisibleSelector, asMoneyRequestReportTransactionsNavigationdoes inside an RHP). They now open on top of the panel as soon as the sync finishes.ROUTES.WORKSPACE_CONNECTIONSexplicitly, as they did before Launch unified Connections: remove the beta and the pages it replaced #105137.Fixed Issues
$ #105713
$ #105722
$ #105690
$ #105786
PROPOSAL: N/A, deploy blocker fix
Tests
Begin link (#105713)
Status tooltip (#105722)
Gusto sync results (#105690)
Back from other panels (#105690)
Links into Connections (#105690)
Offline tests
QA Steps
Same as tests. The Begin link test and the Home Fix step only apply to iOS and Android. The tooltip test only applies to web and desktop.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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: mWeb Chrome
iOS: Native
Begin.link.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
Gusto_RHP_BUG.mp4
toltip.mp4