[No QA] Revise prerequisites for Expensify Card setup#91773
Conversation
Updated prerequisites for setting up the Expensify Card to include company domain requirement. Tracking - Expensify/Expensify#638775 (comment)
Added a link to the guide for updating the primary login email.
This comment has been minimized.
This comment has been minimized.
HelpDot Documentation ReviewOverall AssessmentThis PR adds a new prerequisite to the Expensify Card setup article, requiring that the user's primary login be an email associated with their company's domain. The change converts a single-line prerequisites statement into a numbered list of two items, improving scannability. The addition is small and informative, but contains two governance violations that should be addressed before merging. Scores Summary
Key Findings
Recommendations
Files Reviewed
Note: This review assessed only the proposed changes in the diff, not pre-existing content in the file. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d281dadf6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
@Christinadobrzyn - Do you want to commit the changes suggested by MelvinBot above? If I do it, then we'll need a third person to merge since the last committer can't merge. Once you do feel free to let me know 1:1 and I can action right away. |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…he-Expensify-Card.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
A preview of your ExpensifyHelp changes have been deployed to https://b2951374.helpdot.pages.dev ⚡️ Updated articles: |
strepanier03
left a comment
There was a problem hiding this comment.
I reviewed the changes and the build, both are solid.
|
@strepanier03 looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
| 1. Your primary login must be an email associated with your company's domain. See this guide on [updating your login](/articles/new-expensify/settings/Update-Email-Address). | ||
| 2. You must be a Workspace Admin and have a U.S. business bank account connected to Expensify. See this guide on [connecting a business bank account](/articles/new-expensify/wallet-and-payments/Connect-a-Business-Bank-Account). |
There was a problem hiding this comment.
Cross-link placement violation: Per HELP_AUTHORING_GUIDELINES.md Section 7 ("Link Formatting"), links must not be placed inside numbered step instructions: "Do not place links inside numbered step instructions. Do not interrupt procedural flow with inline links."
These prerequisites are formatted as a numbered list with inline links, which violates this rule. Consider restructuring so the links appear after the numbered list, for example:
**Prerequisites:**
1. Your primary login must be an email associated with your company's domain.
2. You must be a Workspace Admin and have a U.S. business bank account connected to Expensify.
Learn how to [update your email address](/articles/new-expensify/settings/Update-Email-Address) or [connect a business bank account](/articles/new-expensify/wallet-and-payments/Connect-a-Business-Bank-Account).
|
|
||
| **Prerequisites:** You must be a workspace admin and have a U.S. business bank account connected to Expensify. See this guide on [connecting a business bank account](https://help.expensify.com/articles/new-expensify/wallet-and-payments/Connect-a-Business-Bank-Account). | ||
| **Prerequisites:** | ||
| 1. Your primary login must be an email associated with your company's domain. See this guide on [updating your login](/articles/new-expensify/settings/Update-Email-Address). |
There was a problem hiding this comment.
Anchor text violation: Per HELP_AUTHORING_GUIDELINES.md Section 7 ("Anchor Text Rules"), avoid vague anchors like "this guide" or "this article." Use descriptive task-based formats instead.
"See this guide on [updating your login]" should use a clearer pattern such as:
- "Learn how to [update your email address]"
This also applies to line 21: "See this guide on [connecting a business bank account]" should become something like "Learn how to [connect a business bank account]".
HelpDot Documentation ReviewOverall AssessmentThis PR adds a new prerequisite (primary login must be a company domain email) and reformats the prerequisites section from a single-line paragraph into a numbered list. It also converts an absolute URL to a relative link, which is a positive compliance fix. Scores Summary
Key FindingsPositive changes:
Issues to address:
Recommendations
Files Reviewed
Note: This review covers only the proposed changes in the diff, not pre-existing content in the file. |
|
🚀 Deployed to staging by https://github.com/strepanier03 in version: 9.3.89-1 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Adding a comment under the prerequisites section that you need to have your email as the primary login and not a phone number to setup the cards. Updating on both New Expensify and Classic articles.
Related to - https://github.com/Expensify/Expensify/issues/638775#issuecomment-4549530685
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.