[No QA] Update Share-a-Business-Bank-Account.md#81350
[No QA] Update Share-a-Business-Bank-Account.md#81350joekaufmanexpensify merged 2 commits intomainfrom
Conversation
|
A preview of your ExpensifyHelp changes have been deployed to https://50755618.helpdot.pages.dev ⚡️ Updated articles: |
|
Gonna actually put this in review so we can get the claude review comments |
docs/articles/new-expensify/wallet-and-payments/Share-a-Business-Bank-Account.md
Show resolved
Hide resolved
|
|
||
| - If you have access to the existing bank account but the new payer doesn’t: | ||
| - You’ll be prompted to **share** the bank account directly from the **Authorized payer** screen. | ||
| - We’ll share the bank account immediately if you proceed. |
There was a problem hiding this comment.
Style violation: Avoid contractions like "We'll" in documentation. Use "We will" instead to maintain a professional tone. This aligns with Expensify's casual yet professional voice.
There was a problem hiding this comment.
Obviated by above change
| - If you have access to the existing bank account but the new payer doesn’t: | ||
| - You’ll be prompted to **share** the bank account directly from the **Authorized payer** screen. | ||
| - We’ll share the bank account immediately if you proceed. | ||
| - You’ll land on a confirmation screen after the share is successful. |
There was a problem hiding this comment.
Readability violation: This bullet point needs a period at the end for consistency with other list items in the document.
| - If **neither** you nor the new selected payer has access: | ||
| - You’ll be shown an error telling you to ask the current payer to share the account. | ||
|
|
||
| These flows ensure you can only set an Authorized Payer who already has bank account access or is granted access during the flow. |
There was a problem hiding this comment.
Readability violation: The phrase "These flows ensure" is vague. Consider being more specific: "This validation process ensures you can only set an Authorized Payer who already has bank account access or receives access during the payer change."
|
|
||
| These flows ensure you can only set an Authorized Payer who already has bank account access or is granted access during the flow. | ||
|
|
||
|
|
There was a problem hiding this comment.
Style violation: Remove the extra blank line here. There should only be one blank line before the separator (---). This maintains consistent formatting throughout the document.
| These flows ensure you can only set an Authorized Payer who already has bank account access or is granted access during the flow. | ||
|
|
||
|
|
||
| --- |
There was a problem hiding this comment.
Structure violation: The separator --- should be removed here. According to the document structure, there should be only one separator before the FAQ section (which already exists at line 43 in the original). This duplicate separator disrupts the document flow.
There was a problem hiding this comment.
Removed the earlier one
|
|
||
| When changing the **Authorized Payer** in a Workspace: | ||
|
|
||
| - If you have access to the existing bank account but the new payer doesn’t: |
There was a problem hiding this comment.
Readability violation: The nested bullet structure with multiple conditions is complex. Consider breaking this into a clearer format:
If you have access but the new payer doesn't:
- You'll be prompted to share the bank account from the Authorized Payer screen
- The account shares immediately when you confirm
- A confirmation screen appears after successful sharing
This improves scannability and makes the flow easier to follow.
HelpDot Documentation ReviewOverall AssessmentThis PR adds a new section documenting the business bank account sharing functionality when changing the Authorized Payer in a workspace, along with two supporting FAQ entries. The documentation is a solid addition that clearly explains a specific user flow. The content is well-structured and informative, though there are opportunities to enhance readability, improve AI discoverability, and strengthen style compliance. Scores Summary
Key FindingsPositive Aspects:
Areas for Improvement:
Critical Items:
RecommendationsPriority 1:
Priority 2: Priority 3: Files Reviewed
Note: This review focuses on documentation quality dimensions. The content accurately reflects the feature functionality as described in the PR. |
|
Went through all the AI review comments 👍 |
clarajones-expensify
left a comment
There was a problem hiding this comment.
looks good to me!
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
Nice, thanks! I'll merge once the feature PR is out. |
|
Feature PR is on staging and passed QA so going ahead with merging this! |
|
🚀 Deployed to staging by https://github.com/joekaufmanexpensify in version: 9.3.41-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|
Explanation of Change
Updating share bank account article to include new feature for sharing bank account when changing workspace payer.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/556023
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)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.