diff --git a/.github/ISSUE_TEMPLATE/Accessibility.md b/.github/ISSUE_TEMPLATE/Accessibility.md index 93a0d0c8cdf1..47d90e0930d9 100644 --- a/.github/ISSUE_TEMPLATE/Accessibility.md +++ b/.github/ISSUE_TEMPLATE/Accessibility.md @@ -3,7 +3,7 @@ name: Accessibility issue template about: A template to follow when creating a new issue for accessibility failures --- -If you haven’t already, check out our [contributing guidelines](https://github.com/Expensify/ReactNativeChat/blob/main/CONTRIBUTING.md) for onboarding and email contributors@expensify.com to request to join our Slack channel! +If you haven’t already, check out our [contributing guidelines](https://github.com/Expensify/ReactNativeChat/blob/main/docs/CONTRIBUTING.md) for onboarding and email contributors@expensify.com to request to join our Slack channel! ___ ## Action Performed: diff --git a/.github/ISSUE_TEMPLATE/Performance.md b/.github/ISSUE_TEMPLATE/Performance.md index 49871a616bc3..ca61e811841a 100644 --- a/.github/ISSUE_TEMPLATE/Performance.md +++ b/.github/ISSUE_TEMPLATE/Performance.md @@ -5,7 +5,7 @@ title: "[Performance] " labels: Engineering, Daily --- -If you haven’t already, check out our [contributing guidelines](https://github.com/Expensify/ReactNativeChat/blob/main/CONTRIBUTING.md) for onboarding and email contributors@expensify.com to request to join our Slack channel! +If you haven’t already, check out our [contributing guidelines](https://github.com/Expensify/ReactNativeChat/blob/main/docs/CONTRIBUTING.md) for onboarding and email contributors@expensify.com to request to join our Slack channel! ___ ## What performance issue do we need to solve? diff --git a/.github/ISSUE_TEMPLATE/Standard.md b/.github/ISSUE_TEMPLATE/Standard.md index 932e77753a9a..cfa79c9af158 100644 --- a/.github/ISSUE_TEMPLATE/Standard.md +++ b/.github/ISSUE_TEMPLATE/Standard.md @@ -4,7 +4,7 @@ about: A standard template to follow when creating a new issue in this repositor labels: AutoAssignerTriage, Daily --- -If you haven’t already, check out our [contributing guidelines](https://github.com/Expensify/ReactNativeChat/blob/main/CONTRIBUTING.md) for onboarding and email contributors@expensify.com to request to join our Slack channel! +If you haven’t already, check out our [contributing guidelines](https://github.com/Expensify/ReactNativeChat/blob/main/docs/CONTRIBUTING.md) for onboarding and email contributors@expensify.com to request to join our Slack channel! ___ ## Action Performed: diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 90f42ac36bca..f5a45bd6f631 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -42,7 +42,7 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] I added steps for Staging and/or Production testing in the `QA steps` section - [ ] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct) - [ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline) -- [ ] I included screenshots or videos for tests on [all platforms](https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#make-sure-you-can-test-on-all-platforms) +- [ ] I included screenshots or videos for tests on [all platforms](https://github.com/Expensify/App/blob/main/docs/CONTRIBUTING.md#make-sure-you-can-test-on-all-platforms) - [ ] I ran the tests on **all platforms** & verified they passed on: - [ ] iOS / native - [ ] Android / native @@ -51,16 +51,16 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] MacOS / Chrome - [ ] MacOS / Desktop - [ ] I verified there are no console errors (if there’s a console error not related to the PR, report it or open an issue for it to be fixed) -- [ ] I followed proper code patterns (see [Reviewing the code](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) +- [ ] I followed proper code patterns (see [Reviewing the code](https://github.com/Expensify/App/blob/main/docs/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) - [ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. `toggleReport` and not `onIconClick`) - [ ] I verified that comments were added to code that is not self explanatory - [ ] I verified that any new or modified comments were clear, correct English, and explained “why” the code was doing something instead of only explaining “what” the code was doing. - [ ] I verified any copy / text shown in the product was added in all `src/languages/*` files - [ ] I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy. - [ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named “index.js”. All platform-specific files are named for the platform the code supports as outlined in the README. - - [ ] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/STYLE.md#jsdocs)) were followed + - [ ] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/docs/STYLE.md#jsdocs)) were followed - [ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers -- [ ] I followed the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md) +- [ ] I followed the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/docs/PR_REVIEW_GUIDELINES.md) - [ ] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like `Avatar`, I verified the components using `Avatar` are working as expected) - [ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests) - [ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such @@ -91,7 +91,7 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] I verified the steps for Staging and/or Production testing are in the `QA steps` section - [ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct) - [ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline) -- [ ] I checked that screenshots or videos are included for tests on [all platforms](https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#make-sure-you-can-test-on-all-platforms) +- [ ] I checked that screenshots or videos are included for tests on [all platforms](https://github.com/Expensify/App/blob/main/docs/CONTRIBUTING.md#make-sure-you-can-test-on-all-platforms) - [ ] I verified tests pass on **all platforms** & I tested again on: - [ ] iOS / native - [ ] Android / native @@ -100,16 +100,16 @@ This is a checklist for PR authors & reviewers. Please make sure to complete all - [ ] MacOS / Chrome - [ ] MacOS / Desktop - [ ] I verified there are no console errors (if there’s a console error not related to the PR, report it or open an issue for it to be fixed) -- [ ] I verified proper code patterns were followed (see [Reviewing the code](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) +- [ ] I verified proper code patterns were followed (see [Reviewing the code](https://github.com/Expensify/App/blob/main/docs/PR_REVIEW_GUIDELINES.md#reviewing-the-code)) - [ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. `toggleReport` and not `onIconClick`). - [ ] I verified that comments were added to code that is not self explanatory - [ ] I verified that any new or modified comments were clear, correct English, and explained “why” the code was doing something instead of only explaining “what” the code was doing. - [ ] I verified any copy / text shown in the product was added in all `src/languages/*` files - [ ] I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy. - [ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named “index.js”. All platform-specific files are named for the platform the code supports as outlined in the README. - - [ ] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/STYLE.md#jsdocs)) were followed + - [ ] I verified the JSDocs style guidelines (in [`STYLE.md`](https://github.com/Expensify/App/blob/main/docs/STYLE.md#jsdocs)) were followed - [ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers -- [ ] I verified that this PR follows the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/PR_REVIEW_GUIDELINES.md) +- [ ] I verified that this PR follows the guidelines as stated in the [Review Guidelines](https://github.com/Expensify/App/blob/main/docs/PR_REVIEW_GUIDELINES.md) - [ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests) - [ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such - [ ] If a new component is created I verified that: diff --git a/.github/workflows/cla.yml b/.github/workflows/cla.yml index 3e004a024dc7..293d6e782a0d 100644 --- a/.github/workflows/cla.yml +++ b/.github/workflows/cla.yml @@ -32,7 +32,7 @@ jobs: PERSONAL_ACCESS_TOKEN : ${{ secrets.CLA_BOTIFY_TOKEN }} with: path-to-signatures: '${{ github.repository }}/cla.json' - path-to-document: 'https://github.com/${{ github.repository }}/blob/main/CLA.md' + path-to-document: 'https://github.com/${{ github.repository }}/blob/main/docs/CLA.md' branch: 'main' remote-organization-name: 'Expensify' remote-repository-name: 'CLA' diff --git a/.github/workflows/preDeploy.yml b/.github/workflows/preDeploy.yml index e8d133a7a033..da1712b5f0fa 100644 --- a/.github/workflows/preDeploy.yml +++ b/.github/workflows/preDeploy.yml @@ -226,7 +226,7 @@ jobs: body: | @${{ steps.getMergedPullRequest.outputs.author }}, Great job getting your first Expensify/App pull request over the finish line! :tada: - I know there's a lot of information in our [contributing guidelines](https://github.com/Expensify/App/blob/main/CONTRIBUTING.md), so here are some points to take note of :memo:: + I know there's a lot of information in our [contributing guidelines](https://github.com/Expensify/App/blob/main/docs/CONTRIBUTING.md), so here are some points to take note of :memo:: 1. Now that your first PR has been merged, you can be hired for another issue. Once you've completed a few issues, you may be eligible to work on more than one job at a time. 2. Once your PR is deployed to our staging servers, it will undergo quality assurance (QA) testing. If we find that it doesn't work as expected or causes a regression, you'll be responsible for fixing it. Typically, we would revert this PR and give you another chance to create a similar PR without causing a regression. diff --git a/CLA.md b/CLA.md index a87035b2d839..9a78922d017c 100644 --- a/CLA.md +++ b/CLA.md @@ -1,20 +1 @@ -## Expensify Individual Contributor License Agreement - -Thank you for your interest in contributing to open source software projects (“Projects”) made available by Expensify Inc or its affiliates (“Expensify”). This Individual Contributor License Agreement (“Agreement”) sets out the terms governing any source code, object code, bug fixes, configuration changes, tools, specifications, documentation, data, materials, feedback, information or other works of authorship that you submit or have submitted, in any form and in any manner, to Expensify in respect of any of the Projects (collectively “Contributions”). If you have any questions respecting this Agreement, please contact reactnative@expensify.com. - -You agree that the following terms apply to all of your past, present and future Contributions. Except for the licenses granted in this Agreement, you retain all of your rights, title and interest in and to your Contributions. - -**Copyright License.** You hereby grant, and agree to grant, to Expensify a non-exclusive, perpetual, irrevocable, worldwide, fully-paid, royalty-free, transferable copyright license to reproduce, prepare derivative works of, publicly display, publicly perform, and distribute your Contributions and such derivative works, with the right to sublicense the foregoing rights through multiple tiers of sublicensees. - -**Patent License.** You hereby grant, and agree to grant, to Expensify a non-exclusive, perpetual, irrevocable, worldwide, fully-paid, royalty-free, transferable patent license to make, have made, use, offer to sell, sell, import, and otherwise transfer your Contributions, where such license applies only to those patent claims licensable by you that are necessarily infringed by your Contributions alone or by combination of your Contributions with the Project to which such Contributions were submitted, with the right to sublicense the foregoing rights through multiple tiers of sublicensees. - -**Moral Rights.** To the fullest extent permitted under applicable law, you hereby waive, and agree not to assert, all of your “moral rights” in or relating to your Contributions for the benefit of Expensify, its assigns, and their respective direct and indirect sublicensees. - -**Third Party Content/Rights.** If your Contribution includes or is based on any source code, object code, bug fixes, configuration changes, tools, specifications, documentation, data, materials, feedback, information or other works of authorship that were not authored by you (“Third Party Content”) or if you are aware of any third party intellectual property or proprietary rights associated with your Contribution (“Third Party Rights”), then you agree to include with the submission of your Contribution full details respecting such Third Party Content and Third Party Rights, including, without limitation, identification of which aspects of your Contribution contain Third Party Content or are associated with Third Party Rights, the owner/author of the Third Party Content and Third Party Rights, where you obtained the Third Party Content, and any applicable third party license terms or restrictions respecting the Third Party Content and Third Party Rights. For greater certainty, the foregoing obligations respecting the identification of Third Party Content and Third Party Rights do not apply to any portion of a Project that is incorporated into your Contribution to that same Project. - -**Representations.** You represent that, other than the Third Party Content and Third Party Rights identified by you in accordance with this Agreement, you are the sole author of your Contributions and are legally entitled to grant the foregoing licenses and waivers in respect of your Contributions. If your Contributions were created in the course of your employment with your past or present employer(s), you represent that such employer(s) has authorized you to make your Contributions on behalf of such employer(s) or such employer(s) has waived all of their right, title or interest in or to your Contributions. - -**No Obligation.** You acknowledge that Expensify is under no obligation to use or incorporate your Contributions into any of the Projects. The decision to use or incorporate your Contributions into any of the Projects will be made at the sole discretion of Expensify or its authorized delegates. - -**Assignment.** You agree that Expensify may assign this Agreement, and all of its rights, obligations and licenses hereunder. - +This file has been moved [here](./docs/CLA.md). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fee06cf7c896..23ca1a953092 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,130 +1 @@ -# Contributing to Expensify -Welcome! Thanks for checking out the new Expensify app and for taking the time to contribute! - -## Getting Started -If you would like to become an Expensify contributor, the first step is to read this document in its entirety. The second step is to review the README guidelines [here](https://github.com/Expensify/App/blob/main/README.md) to understand our coding philosophy and for a general overview of the code repository (i.e. how to run the app locally, testing, storage, our app philosophy, etc). Please read both documents before asking questions, as it may be covered within the documentation. - -#### Test Accounts -You can create as many accounts as needed in order to test your changes directly from [the app](https://new.expensify.com/). An initial account can be created when logging in for the first time, and additional accounts can be created by opening the "New Chat" or "Group Chat" pages via the Global Create menu, inputting a valid email or phone number, and tapping the user's avatar. - -**Note**: When testing chat functionality in the app please do this between accounts you or your fellow contributors own - **do not test chatting with Concierge**, as this diverts to our customer support team. Thank you. - -##### Generating Multiple Test Accounts -You can generate multiple test accounts by using a `+` postfix, for example if your email is test@test.com, you can create multiple New Expensify accounts connected to the same email address by using test+123@test.com, test+456@test.com, etc. - -#### Working on beta features - -Some features are locked behind beta flags while development is ongoing. As a contributor you can work on these beta features locally by overriding the [`Permissions.canUseAllBetas` function](https://github.com/Expensify/App/blob/5e268df7f2989ed04bc64c0c86ed77faf134554d/src/libs/Permissions.js#L10-L12) to return `true`. - -## Code of Conduct -This project and everyone participating in it is governed by the Expensify [Code of Conduct](https://github.com/Expensify/App/blob/main/CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code. Please report unacceptable behavior to [contributors@expensify.com](mailto:contributors@expensify.com). - -## Restrictions -At this time, we are not hiring contractors in Crimea, North Korea, Russia, Iran, Cuba, or Syria. - -## Asking Questions -If you have any general questions, please ask in the #expensify-open-source Slack channel. To request an invite to the channel, just email contributors@expensify.com with the subject `Slack Channel Invite` and include a link to your Upwork profile. We'll send you an invite! Note: The Expensify team will not be able to respond to direct messages in Slack. - -If you are hired for an Upwork job and have any job-specific questions, please ask in the GitHub issue or pull request. This will ensure that the person addressing your question has as much context as possible. - -## Reporting Vulnerabilities -If you've found a vulnerability, please email security@expensify.com with the subject `Vulnerability Report` instead of creating an issue. - -## Payment for Contributions -We hire and pay external contributors via Upwork.com. If you'd like to be paid for contributing, please create an Upwork account, apply for an available job in [GitHub](https://github.com/Expensify/App/issues?q=is%3Aopen+is%3Aissue+label%3A%22Help+Wanted%22), and finally apply for the job in Upwork once your proposal gets selected in GitHub. If you think your compensation should be increased for a specific job, you can request a reevaluation by commenting in the Github issue where the Upwork job was posted. - -Payment for your contributions will be made no less than 7 days after the pull request is deployed to production to allow for regression testing. If a regression occurs, payment will be issued 7 days after all regressions are fixed. If you have not received payment after 8 days of the PR being deployed to production and there being no regressions, please email contributors@expensify.com referencing the GH issue and your GH handle. - -New contributors are limited to working on one job at a time, however experienced contributors may work on numerous jobs simultaneously. - -## Finding Jobs -There are two ways you can find a job that you can contribute to: - -#### Finding a job that Expensify posted -This is the most common scenario for contributors. The Expensify team posts new jobs to the Upwork job list [here](https://www.upwork.com/ab/jobs/search/?q=Expensify%20React%20Native&sort=recency&user_location_match=2) (you must be signed in to Upwork to view jobs). Each job in Upwork has a corresponding GitHub issue, which will include instructions to follow. You can also view open jobs by searching for issues in GitHub with the [`Help Wanted` label](https://github.com/Expensify/App/issues?q=is%3Aopen+is%3Aissue+label%3A%22Help+Wanted%22). - -#### Proposing a job that Expensify hasn’t posted - -It’s possible that you found a bug or enhancement that we haven’t posted to the [GitHub repository](https://github.com/Expensify/App/issues?q=is%3Aissue). This is an opportunity to propose a job, and (optionally) a solution. If it's a valid job proposal that we choose to implement by deploying it to production — either internally or via an external contributor — then we will compensate you $250 for identifying and proposing the improvement. -- Note: If you get assigned the job you proposed **and** you complete the job, this $250 for identifying the improvement is *in addition to* the reward you will be paid for completing the job. -- Note about proposed improvements: Expensify has the right not to pay the $250 reward if the suggested improvement is already planned. Currently, Expensify plans to implement all features of the old Expensify app in New Expensify. - -Please follow these steps to propose a job: - -1. Check to ensure an issue does not already exist for this topic in the [New Expensify Issue list](https://github.com/Expensify/App/issues). Please use your best judgement by searching for similar titles and issue descriptions. -2. If your bug or enhancement matches an existing issue, please feel free to comment on that GitHub issue with your findings if you think it will help solve the issue. -4. If there is no existing GitHub issue or Upwork job, check if the issue is happening on prod (as opposed to only happening on dev) -5. If the issue is just in dev then it means it's a new issue and has not been deployed to production. In this case, you should try to find the offending PR and comment in the issue tied to the PR and ask the assigned users to add the `DeployBlockerCash` label. If you can't find it, follow the reporting instructions in the next item, but note that the issue is a regression only found in dev and not in prod. -6. If the issue happens in main, staging or production then report the issue(s) in the [#expensify-open-source](https://github.com/Expensify/App/blob/main/CONTRIBUTING.md#asking-questions) Slack channel, prefixed with `BUG:` or `Feature Request:`. Please use the templates for bugs and feature requests that are bookmarked in #expensify-open-source. View [this guide](https://github.com/Expensify/App/blob/main/HOW_TO_CREATE_A_PLAN.md) for help creating a plan when proposing a feature request. -7. After review in #expensify-open-source, if you've provided a quality proposal that we choose to implement, a GitHub issue will be created and your Slack handle will be included in the original post after `Issue reported by:` -8. If an external contributor other than yourself is hired to work on the issue, you will also be hired for the same job in Upwork. No additional work is needed. If the issue is fixed internally, a dedicated job will be created to hire and pay you after the issue is fixed. -9. Payment will be made 7 days after code is deployed to production if there are no regressions. If a regression is discovered, payment will be issued 7 days after all regressions are fixed. - ->**Note:** Our problem solving approach at Expensify is to focus on high value problems and avoid small optimizations with results that are difficult to measure. We also prefer to identify and solve problems at their root. Given that, please ensure all proposed jobs fix a specific problem in a measurable way with evidence so they are easy to evaluate. Here's an example of a good problem/solution: -> ->**Problem:** The app start up time has regressed because we introduced "New Feature" in PR #12345 and is now 1042ms slower because `SomeComponent` is re-rendering 42 times. -> ->**Solution:** Start up time will perceptibly decrease by 1042ms if we prevent the unnecessary re-renders of this component. - -## Working on Expensify Jobs -*Reminder: For technical guidance please refer to the [README](https://github.com/Expensify/App/blob/main/README.md)*. - -## Posting Ideas -Additionally if you want to discuss an idea with the community without having a P/S statement yet, you can post it in #expensify-open-source with the prefix `IDEA:`. All ideas to build the future of Expensify are always welcome! i.e.: "`IDEA:` I don't have a P/S for this yet, but just kicking the idea around... what if we [insert crazy idea]?". - -#### Make sure you can test on all platforms -* Expensify requires that you can test the app on iOS, MacOS, Android, Web, and mWeb. -* You'll need a Mac to test the iOS and MacOS app. -* In case you don't have one, here's a helpful [document](https://github.com/Expensify/App/blob/main/TESTING_MACOS_AND_IOS.md) on how you might test all platforms on a Windows/Linux device. - -#### Check GitHub for existing proposals from other users - -1. Expensify reviews all solution proposals on a first come first serve basis. If you see other contributors have already proposed a solution, you can still provide a solution proposal and we will review it. We look for the earliest provided, best proposed solution that addresses the job. - -#### Make sure you can reproduce the problem -2. Use your test account(s) to reproduce the problem by following the steps in the GitHub issue. -3. If you cannot reproduce the problem, pause on this step and add a comment to the issue explaining where you are stuck or that you don't think the issue can be reproduced. - -#### Propose a solution for the job -4. After you reproduce the issue, make a proposal for your solution and post it as a comment in the corresponding GitHub issue (linked in the Upwork job). Your solution proposal should include a brief written technical explanation of the changes you will make. Include "Proposal" as the first word in your comment. - - Note: Issues that have not had the `External` label applied have not yet been approved for implementation. This means, if you propose a solution to an issue without the `External` label (which you are allowed to do) it is possible that the issue will be fixed internally. If the `External` label has not yet been applied, Expensify has the right to use your proposal to fix said issue, without providing compensation for your solution. This process covers the very rare instance where we need or want to fix an issue internally. - - Note: Before submitting a proposal on an issue, be sure to read any other existing proposals. Any new proposal should be substantively different from existing proposals. -5. Pause at this step until someone from the Contributor-Plus team and / or someone from Expensify provides feedback on your proposal (do not create a pull request yet). -6. If your solution proposal is accepted by the Expensify engineer assigned to the issue, Expensify will hire you on Upwork and assign the GitHub issue to you. - -#### Begin coding your solution in a pull request -7. When you are ready to start, fork the repository and create a new branch. -8. Before you begin writing any code, please be aware that we require all commits to be [signed](https://docs.github.com/en/github/authenticating-to-github/signing-commits). The easiest way to do that is to [generate a new GPG key](https://docs.github.com/en/github/authenticating-to-github/generating-a-new-gpg-key) and [add it to your GitHub account](https://docs.github.com/en/github/authenticating-to-github/adding-a-new-gpg-key-to-your-github-account). Once you've done that, you can automatically sign all your commits by adding the following to your `.gitconfig`: - ``` - [commit] - gpgsign = true - [user] - email = - name = - signingkey = - [gpg] - program = gpg - ``` -9. [Open a pull request](https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork), and make sure to fill in the required fields. -10. An Expensify engineer and a member from the Contributor-Plus team will be assigned to your pull request automatically to review. -11. Daily updates on weekdays are highly recommended. If you know you won’t be able to provide updates for > 1 week, please comment on the PR or issue how long you plan to be out so that we may plan accordingly. We understand everyone needs a little vacation here and there. Any issue that doesn't receive an update for 1 full week may be considered abandoned and the original contract terminated. - -#### Submit your pull request for final review -12. When you are ready to submit your pull request for final review, make sure the following checks pass: - 1. CLA - You must sign our [Contributor License Agreement](https://github.com/Expensify/App/blob/main/CLA.md) by following the CLA bot instructions that will be posted on your PR - 2. Tests - All tests must pass before a merge of a pull request - 3. Lint - All code must pass lint checks before a merge of a pull request -13. Please never force push when a PR review has already started (because this messes with the PR review history) -14. Please pay attention to the pull request template, especially to how we link PRs with issues they fix. Make sure you don't use GitHub keywords such as `fixes` in your PR description, as this can break our current automated steps for issue management. Follow the PR template format carefully. -15. Upon submission of a PR, please include a numbered list of explicit testing steps for each platform (Web, Desktop, iOS, Android, and Mobile Web) to confirm the fix works as expected and there are no regressions. -16. Please add a screenshot of the app running on each platform (Web, Desktop, iOS, Android, Mobile Web). - -#### Timeline expectations and asking for help along the way -- If you have made a change to your pull request and are ready for another review, leave a comment that says "Updated" on the pull request itself. -- Please keep the conversation in GitHub, and do not ping individual reviewers in Slack or Upwork to get their attention. -- Pull Request reviews can sometimes take a few days. If your pull request has not been addressed after four days please let us know via the #expensify-open-source Slack channel. -- On occasion, our engineers will need to focus on a feature release and choose to place a hold on the review of your PR. Depending on the hold length, our team will decide if a bonus will be applied to the job. - -#### Important note about JavaScript Style -- Read our official [JavaScript and React style guide](STYLE.md). Please refer to our Style Guide before asking for a review. -- We have nothing against Prettier or any other automatic style fixers, but we generally don't use them here at Expensify. Do not use Prettier. The style changes these tools enforce don't always align with the ones we recommend and require in our eslint configs and can result in uncessary changes for our reviewers. Ignoring this advice will ultimately make your changes take longer to review as we will ask you to undo any style changes that are not related to the important changes you are making. +This file has been moved [here](./docs/CONTRIBUTING.md). diff --git a/FORMS.md b/FORMS.md index d80b2dff3310..c6d36ff480be 100644 --- a/FORMS.md +++ b/FORMS.md @@ -1,213 +1 @@ -# Creating and Using Forms - -This document lists specific guidelines for using our Form component and general forms guidelines. - -## General Form UI/UX - -### Labels, Placeholders, & Hints - -Labels are required for each input and should clearly mark the field. Optional text may appear below a field when a hint, suggestion, or context feels necessary. If validation fails on such a field, its error should clearly explain why without relying on the hint. Inline errors should always replace the microcopy hints. Placeholders should not be used as it’s customary for labels to appear inside form fields and animate them above the field when focused. - -![hint](https://user-images.githubusercontent.com/22219519/156266779-72deaf42-832c-453c-a5c2-1b2073b8b3b7.png) - -Labels and hints are enabled by passing the appropriate props to each input: - -```jsx - -``` - -### Character Limits - -If a field has a character limit we should give that field a max limit. This is done by passing the maxLength prop to TextInput. - -```jsx - -``` -Note: We shouldn't place a max limit on a field if the entered value can be formatted. eg: Phone number. -The phone number can be formatted in different ways. - -- 2109400803 -- +12109400803 -- (210)-940-0803 - -### Native Keyboards - -We should always set people up for success on native platforms by enabling the best keyboard for the type of input we’re asking them to provide. See [keyboardType](https://reactnative.dev/docs/0.64/textinput#keyboardtype) in the React Native documentation. - -We have a couple of keyboard types [defined](https://github.com/Expensify/App/blob/572caa9e7cf32a2d64fe0e93d171bb05a1dfb217/src/CONST.js#L357-L360) and should be used like so: - -```jsx - -``` - -### Autofill Behavior - -Forms should autofill information whenever possible i.e. they should work with browsers and password managers auto complete features. - -As a best practice we should avoid asking for information we can get via other means e.g. asking for City, State, and Zip if we can use Google Places to gather information with the least amount of hassle to the user. - -Browsers use the name prop to autofill information into the input. Here's a [reference](https://developers.google.com/web/fundamentals/design-and-ux/input/forms#recommended_input_name_and_autocomplete_attribute_values) for available values for the name prop. - -```jsx - -``` - -### Focus and Tab Behavior - -All forms should define an order in which the inputs should be filled out, and using tab / shift + tab to navigate through the form should traverse the inputs in that order/reversed order, respectively. In most cases this can be achieved by composition, i.e. rendering the components in the correct order. If we come across a situation where composition is not enough, we can: - -1. Create a local tab index state -2. Assign a tab index to each form input -3. Add an event listener to the page/component we are creating and update the tab index state on tab/shift + tab key press -4. Set focus to the input with that tab index. - -Additionally, pressing the enter key on any focused field should submit the form. - -Note: This doesn't apply to the multiline fields. To keep the browser behavior consistent, pressing enter on the multiline should not be intercepted. It should follow the default browser behavior (such as adding a newline). - -### Modifying User Input on Change - -User input that may include optional characters (e.g. (, ), - in a phone number) should never be restricted on input, nor be modified or formatted on blur. This type of input jacking is disconcerting and makes things feel broken. - -Instead we will format and clean the user input internally before using the value (e.g. making an API request where the user will never see this transformation happen). Additionally, users should always be able to copy/paste whatever characters they want into fields. - -To give a slightly more detailed example of how this would work with phone numbers we should: - -1. Allow any character to be entered in the field. -2. On blur, strip all non-number characters (with the exception of + if the API accepts it) and validate the result against the E.164 regex pattern we use for a valid phone. This change is internal and the user should not see any changes. This should be done in the validate callback passed as a prop to Form. -3. On submit, repeat validation and submit with the clean value. - -### Form Drafts - -Form inputs will NOT store draft values by default. This is to avoid accidentely storing any sensitive information like passwords, SSN or bank account information. We need to explicitly tell each form input to save draft values by passing the shouldSaveDraft prop to the input. Saving draft values is highly desireable and we should always try to save draft values. This way when a user continues a given flow they can easily pick up right where they left off if they accidentally exited a flow. Inputs with saved draft values [will be cleared when a user logs out](https://github.com/Expensify/App/blob/aa1f0f34eeba5d761657168255a1ae9aebdbd95e/src/libs/actions/SignInRedirect.js#L52) (like most data). Additionally we should clear draft data once the form is successfully submitted by calling `Onyx.set(ONYXKEY.FORM_ID, null)` in the onSubmit callback passed to Form. - -```jsx - -``` - -## Form Validation and Error handling - -### Validate on Blur and Submit - -Each individual form field that requires validation will have its own validate test defined. When the form field loses focus (blur) we will run that validate test and show feedback. A blur on one field will not cause other fields to validate or show errors unless they have already been blurred. - -All form fields will additionally be validated when the form is submitted. Although we are validating on blur this additional step is necessary to cover edge cases where forms are auto-filled or when a form is submitted by pressing enter (i.e. there will be only a ‘submit’ event and no ‘blur’ event to hook into). - -The Form component takes care of validation internally and the only requirement is that we pass a validate callback prop. The validate callback takes in the input values as argument and should return an object with shape `{[inputID]: errorMessage}`. Here's an example for a form that has two inputs, `routingNumber` and `accountNumber`: - -```js -function validate(values) { - const errors = {}; - if (!values.routingNumber) { - errors.routingNumber = props.translate(CONST.ERRORS.ROUTING_NUMBER); - } - if (!values.accountNumber) { - errors.accountNumber = props.translate(CONST.ERRORS.ACCOUNT_NUMBER); - } - return errors; -} -``` - -For a working example, check [Form story](https://github.com/Expensify/App/blob/aa1f0f34eeba5d761657168255a1ae9aebdbd95e/src/stories/Form.stories.js#L63-L72) - -### Highlight Fields and Inline Errors - -Individual form fields should be highlighted with a red error outline and present supporting inline error text below the field. Error text will be required for all required fields and optional fields that require validation. This will keep our error handling consistent and ensure we put in a good effort to help the user fix the problem by providing more information than less. - -![error](https://user-images.githubusercontent.com/22219519/156267035-af40fe93-da27-4e16-bc55-b7cd40b0f1f2.png) - -### Multiple Types of Errors for Individual Fields - -Individual fields should support multiple messages depending on validation e.g. a date could be badly formatted or outside of an allowable range. We should not only say “Please enter a valid date” and instead always tell the user why something is failing if we can. The Form component supports an infinite number of possible error messages per field and they are displayed simultaneously if multiple validations fail. - -### Form Alerts - -When any form field fails to validate in addition to the inline error below a field, an error message will also appear inline above the submit button indicating that some fields need to be fixed. A “fix the errors” link will scroll the user to the first input that needs attention and focus on it (putting the cursor at the end of the existing value). By default, on form submit and when tapping the “fix the errors” link we should scroll the user to the first field that needs their attention. - -![form-alert](https://user-images.githubusercontent.com/22219519/156267105-861fbe81-32cc-479d-8eff-3760bd0585b1.png) - -### Handling Server Errors - -Server errors related to form submission should appear in the Form Alert above the submit button. They should not appear in growls or other kinds of alerts. Additionally, as best practice moving forward server errors should never solely do the work that frontend validation can also do. This means that any error that can be validated in the frontend should be validated in the frontend and backend. - -Note: This is not meant to suggest that we should avoid validating in the backend if the frontend already validates. - -Note: There are edge cases where some server errors will inevitably relate to specific fields in a form with other fields unrelated to that error. We had trouble coming to a consensus on exactly how this edge case should be handled (e.g. show inline error, clear on blur, etc). For now, we will show the server error in the form alert and not inline (so the “fix the errors” link will not be present). In those cases, we will still attempt to inform the user which field needs attention, but not highlight the input or display an error below the input. We will be on the lookout for our first validation in the server that could benefit from being tied to a specific field and try to come up with a unified solution for all errors. - -## Form Submission -### Submit Button Disabling - -Submit buttons shall not be disabled or blocked from being pressed in most cases. We will allow the user to submit a form and point them in the right direction if anything needs their attention. - -The only time we won’t allow a user to press the submit button is when we have submitted the form and are waiting for a response (e.g. from the API). In this case we will show a loading indicator and additional taps on the submit button will have no effect. This is handled by the Form component and will also ensure that a form cannot be submitted multiple times. - -## Using Form.js - -The example below shows how to use [Form.js](https://github.com/Expensify/App/blob/c5a84e5b4c0b8536eed2214298a565e5237a27ca/src/components/Form.js) in our app. You can also refer to [Form.stories.js](https://github.com/Expensify/App/blob/c5a84e5b4c0b8536eed2214298a565e5237a27ca/src/stories/Form.stories.js) for more examples. - -```jsx -function validate(values) { - const errors = {}; - if (!values.routingNumber) { - errors.routingNumber = 'Please enter a routing number'; - } - if (!values.accountNumber) { - errors.accountNumber = 'Please enter an account number'; - } - return errors; -} - -function onSubmit(values) { - setTimeout(() => { - alert(`Form submitted!`); - FormActions.setIsSubmitting('TestForm', false); - }, 1000); -} - -
- // Wrapping TextInput in a View to show that Form inputs can be nested in other components - - - - - -``` - -### Props provided to Form inputs - -The following prop is available to form inputs: - -- inputID: An unique identifier for the input. - -Form.js will automatically provide the following props to any input with the inputID prop. - -- ref: A React ref that must be attached to the input. -- defaultValue: The input default value. -- errorText: The translated error text that is returned by validate for that specific input. -- onBlur: An onBlur handler that calls validate. -- onInputChange: An onChange handler that saves draft values and calls validate for that input (inputA). Passing an inputID as a second param allows inputA to manipulate the input value of the provided inputID (inputB). +This file has been moved [here](./docs/FORMS.md). diff --git a/HOW_TO_CREATE_A_PLAN.md b/HOW_TO_CREATE_A_PLAN.md index cb1f8b4a7f40..ae0616ed6157 100644 --- a/HOW_TO_CREATE_A_PLAN.md +++ b/HOW_TO_CREATE_A_PLAN.md @@ -1,45 +1 @@ -## How to create a plan - Contributor version - -### Preamble -If you’re new to working with Expensify, it’s important to know the philosophy behind the design of Expensify’s products and the processes that wrap around them. -- Problems are identified and defined before solutions are proposed -- Features are not added to satisfy personal preferences -- Features are added thoughtfully and minimally - -It’s also important to be familiar with our existing documentation including [CONTRIBUTING.md](https://github.com/Expensify/App/blob/main/CONTRIBUTING.md) and [README.md](https://github.com/Expensify/App/blob/main/README.md) - -## Step 1: Define the problem -What is the problem that needs solving? -- Be broad -- Define who experiences the problem -- Does this affect the customer experience? How so? -- How many customers would be affected by this? -- Define why it is a problem -- Resist the temptation to reverse-engineer a Solution into a matching Problem. (ie. Solution: Let's get a car! Problem: We don’t have a car.) -- There may be more than one problem, so define all of them -- Keep the language simple - Reduce every word down to its most plain and straightforward manner. Avoid jargon. If you use abbreviations, include the full spelling the first time. -- Tear apart your statements and have others tear them apart for you until everybody agrees unambiguously as to the nature of the problem. If needed, continue the discussion in #expensify-open-source. - -Identify which problems are actually independent and can be solved by themselves, versus which are actually a series of dependent but otherwise separate problems that can be solved one at a time. - -The broader and better contextualized the problem definition, the greater opportunity for discovering radical, outside-of-the-box solutions. The odds of achieving something are dramatically improved by clearly defining what it is. Take at least as much care perfecting your problem definition as you do the solution, because the perfect solution to the wrong problem is typically an awful solution to the right one. - -## Step 2: Workshop the solutions -Draft as many solutions as you can for each problem, including any existing workarounds - “Do nothing” is always a potential solution. Be as creative or audacious as you want when workshopping potential solutions. - -Once you have your solutions, it’s time to decide on the preferred solution (alternate solutions may be included at a later stage). Aspects that may promote one solution to “preferred” status: -- ROI (return on investment) - the current value of the problem, versus the cost of your solution -- Work involved - is one solution just as effective but would take half the time to create? -- Adding or moving “things” - does one solution have less consumer impact? -- Future-proofing - does one solution have more longevity or pave the way for future development? -- Independence - does a solution rely on a different problem to be solved first? Does it rely on another piece to be done later? - -If you are finding the solution to be difficult, go back and beat harder on the problem to break it up into smaller pieces. Keep repeating until you have a general list of prioritized stages, with early stages solving the dependencies required by later stages, each of which is extremely well defined, with a reasonably obvious preferred solution. - -## Step 3: Write out each problem and solution (P/S statement) -Have a trusted peer or two proof your P/S statement and help you ensure it is well-defined. If you're in need of a peer to proof, post in #expensify-open-source to ask for help. Refine it and then share with another peer or two until you have a clear, understandable P/S statement. The more complex the problem and solution, the more people should review it. Keep going back to step 1 if needed. - -## Step 4: Propose it -Once you have a well-defined problem and solution, post your problem and solution as separate comments in ​​#expensify-open-source in Slack and be prepared to answer questions and defend your choices. Also, be prepared to hear better solutions that may completely change your P/S statement. - -Depending on the complexity of the P/S Statement, an internal Design Doc may be required (Expensify employees will take it from there). +This file has been moved [here](./docs/HOW_TO_CREATE_A_PLAN.md). diff --git a/OfflineUX.md b/OfflineUX.md index 801523a6aa63..a11c1d72f0bc 100644 --- a/OfflineUX.md +++ b/OfflineUX.md @@ -1,133 +1 @@ -#### Offline UX Patterns - -### Contents -* [Motivation & Philosophy](#motivation-&-philosophy) -* [UX Pattern Flowchart](#ux-pattern-flowchart) -* [Answering Questions on the Flow Chart](#answering-questions-on-the-flowchart) -* [Description of the Patterns](#description-of-the-patterns) - - [None - No Offline Behavior](#none---no-offline-behavior) - - [A - Optimistic Without Feedback](#a---optimistic-without-feedback) - - [B - Optimistic With Feedback](#b---optimistic-with-feedback) - - [C - Blocking Form](#c---blocking-form) - - [D - Full Page Blocking](#d---full-page-blocking) - -### Motivation & Philosophy - -Understanding the offline behavior of our app is vital to becoming a productive contributor to the Expensify codebase. Our mission is to support our users in every possible environment, and often our app is used in places where a stable internet connection is not guaranteed. - -The most important concept to keep in mind while reading this document is that we want to allow users to do as much as possible when offline. At first, this might seem impossible because almost everything the user can touch in our app is related to an API request. However, in many cases, we can save that API request and assume it will succeed when the user is back online. We then allow the user to proceed as if their request already succeeded. We call this an optimistic response. Here, we use the word **optimistic** to indicate that we're confident the request will succeed when the user is online, and we know what that successful response will look like. - -
-Example: Pinning a chat - -When a user clicks the pin button on a chat, two things should happen. - -1. **API Request:** We send a request to the API to ensure the change is saved in the database. This way the chat is pinned on all the user's devices, and will remain pinned even if they leave the app and come back. - -2. **UI Changes:** The chat should go to the top of the list with the other pinned chats, and the pin button should look darker than it did before. This is visual feedback that clicking the pin button worked. - -If the user is offline, we don't need to wait for the API request to finish before doing all that visual stuff because this particular API request has almost no way of failing, and we know what the server will return in advance. That means we can safely assume that when we retry the command, it will succeed and that's why we let the user continue using the app as if the action succeeded. - -
- -The example we just looked at is nice and simple, but some actions should not use this approach (example: requesting money from another user). For these types of actions, we can't simply proceed as if the request already finished. Here are some reasons why: - -1. We don't know _how_ to proceed because of a lack of information (often the server returns data that we wouldn't be able to guess the content of). - -2. We may be able to guess what a successful request would look like, but we don't want to misguide the user into believing their action was completed. For example, we don't want the user to believe that a financial transaction has been made when it actually hasn't. - -To handle problems like this, we have developed offline UX patterns and guidance on when to use them. Every feature of this application should fit into one of these patterns. - -### Descriptions of the UX Patterns - -# None - No Offline Behavior - -There’s no specific UI for this case. The feature either looks totally normal and works as expected (because it doesn’t need the server to function) or the feature looks like it did whenever connection was lost. - -**Used when…** - - there is no interaction with the server in any way - - or data is READ from the server and does not need to show up-to-date data. The user will see stale data until the new data is put into Onyx and then the view updates to show the new data. - - **How to implement:** Use [`API.read()`](https://github.com/Expensify/App/blob/3493f3ca3a1dc6cdbf9cb8bd342866fcaf45cf1d/src/libs/API.js#L53-L55). - -# A - Optimistic Without Feedback Pattern - -This is the pattern where we queue the request to be sent when the user is online and we continue as if the request succeeded. - -**Used when…** - - the user should be given instant feedback and - - the user does not need to know when the change is done on the server in the background - -**How to implement:** Use [`API.write()`](https://github.com/Expensify/App/blob/3493f3ca3a1dc6cdbf9cb8bd342866fcaf45cf1d/src/libs/API.js#L7-L28) to implement this pattern. For this pattern we should only put `optimisticData` in the options. We don't need successData or failData as we don't care what response comes back at all. - -# B - Optimistic WITH Feedback Pattern -This pattern queues the API request, but also makes sure that the user is aware that the request hasn’t been sent yet **when the user is offline**. When the user is online, the feature should just look like it succeeds immediately (we dont want the offline UI to flicker on and off when the user is online). - -**Used when…** - - the user needs feedback that data will be sent to the server later -This is a minority use case at the moment, but INCREDIBLY HELPFUL for the user, so proceed with cautious optimism. - -**How to implement:** Use API.write() to implement this pattern. Optimistic data should include some pending state for the action that is reflected in the UI. Success/failure data should revert the pending state and/or set a failure state accordingly. - -# C - Blocking Form UI Pattern -This pattern greys out the submit button on a form and does not allow the form to be submitted. We also show a "You appear offline" message near the bottom of the screen. Importantly, we _do_ let the user fill out the form fields. That data gets saved locally so they don’t have to fill it out again once online. - -**Used when…** - - a form is used to make a WRITE request to the server and - - server has to do some validation of the parameters that can’t be done in the client or - - server response will be unknown so it cannot be done optimistically - - If the request is moving money - -**How to implement:** Use the `` component. This pattern should use the `API.write()` method. - -# D - Full Page Blocking UI Pattern -This pattern blocks the user from interacting with an entire page. - -**Used when…** - - blocking READ is being performed. This occurs when the data that a user sees cannot be stale data and the data can only be displayed after fetching it from the server (eg. Plaid's list of bank accounts) - - the app is offline and the data cannot be fetched - - an error occurs when fetching the data and the user needs instructions on what to do next -This should only be used in the most extreme cases when all other options have been completely and utterly exhausted - -**How to implement:** Wrap the component you're working on in a `` component. - -### UX Pattern Flow Chart - -The following flowchart can be used to determine which UX pattern should be used. - -![New Expensify Data Flow Chart](/web/OfflineUX_Patterns_Flowchart.png) - -### Answering Questions on the Flow Chart - -The numbers in this section correlate to the numbers in each decision box above (the diamond shapes). - -1. Does the feature interact with the server? - -If you're changing an existing feature, you can open the network tab of dev tools to see if any network requests are being made when you use the feature. If network requests are being made, the answer to this question is YES. Note: Sometimes you may see requests that happen to fire at the same time as the feature you're working on, so be sure to double check. -If you're making a new feature, think about whether any data would need to be retrieved or stored from anywhere other than the local device. If data needs to be stored to or retrieved from the server, then the answer is YES. - -2. What type of request is being made? - -If there's new data being saved on the server, you're making a WRITE request. If you're retrieving existing data from the server, you're making a READ request. If both things are happening, that's a WRITE request. - -3. Is it OK for the user to see stale data? - -Example: The payment method list. We don't want the user to see a payment method that we no longer support, not even while the payment methods are being loaded from the server (or while the user is offline). Therefore, we answer NO, which leads us to the blocking UI. This way the user won't see stale data while we load the payment methods. - -4. Is the UI a form? - -An easy way to tell if something is a form is to try and find a submit button. If a submit button is present or if the user is filling out form inputs, answer YES to this question. - -5. Can the server response be anticipated? - -Answer NO if there is data coming back from the server that we can't know (example: a list of bank accounts from Plaid, input validation that the server must perform). Answer YES if we can know what the response from the server would be. - -6. Is there validation done on the server that can't be done on the front end? - -If there is some validation happening on the server that needs to happen before the feature can work, then we answer YES to this question. Remember, this is referring to validation that cannot happen on the front end (e.g. reusing an existing password when resetting a password). For example, if we want to set up a bank account then our answer to this question is YES (because we can’t suggest to the user that their request succeeded when really it hasn’t been sent yet–their card wouldn’t work!) - -This question can be tricky, so if you're unsure, please ask a question in the #expensify-open-source slack room and tag @contributor-management-engineering. - -7. Does the user need to know if the action was successful? - -Think back to the pinning example from above: the user doesn’t need to know that their pinned report's NVP has been updated. To them the impact of clicking the pin button is that their chat is at the top of the LHN. It makes no difference to them if the server has been updated or not, so the answer would be NO. Now let’s consider sending a payment request to another user. In this example, the user needs to know if their request was actually sent, so our answer is YES. +This file has been moved [here](./docs/OFFLINE_UX.md). diff --git a/PERFORMANCE.md b/PERFORMANCE.md index a80b52d5885f..2356a29a8a17 100644 --- a/PERFORMANCE.md +++ b/PERFORMANCE.md @@ -1,94 +1 @@ -# React Performance Tips - -- Always test performance with the production build as development mode is not optimized. -- Use [`PureComponent`](https://reactjs.org/docs/react-api.html#reactpurecomponent), [`React.memo()`](https://reactjs.org/docs/react-api.html#reactmemo), and [`shouldComponentUpdate()`](https://reactjs.org/docs/react-component.html#shouldcomponentupdate) to prevent re-rendering expensive components. -- Using a combination of [React DevTools Profiler](https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi?hl=en) and [Chrome Dev Tools Performance Timing](https://calibreapp.com/blog/react-performance-profiling-optimization) can help identify unnecessary re-renders. Both tools can be used to time an interaction like the app starting up or navigating to a new screen. -- Watch out for [very large lists](https://reactnative.dev/docs/optimizing-flatlist-configuration) and things like `Image` components re-fetching images on render when a remote uri did not change. -- Avoid the temptation to over-optimize. There is added cost in both code complexity and performance when adding checks like `shouldComponentUpdate()`. Be selective about when you use this and make sure there is a measureable difference before proposing the change. As a very general rule it should be measurably faster to run logic to avoid the re-render (e.g. do a deep comparison) than it would be to let React take care of it without any extra intervention from us. -- Use caution when adding subscriptions that might re-render very large trees of components e.g. subscribing to state that changes often (current report, current route, etc) in the app root. -- Avoid using arrow function callbacks in components that are expensive to re-render. React will re-render this component since each time the parent renders it creates a new instance of that function. **Alternative:** Bind the method in the constructor instead. - -## Tools - -### Chrome Dev Tools > Performance > Timing (Web Only) - -- Profiling in Chrome Dev Tools performance tab in the "Timing" section -- This will show various components and how long they took to render. It can be a little intense to dig through it all at first, but the more time you spend with it the easier it gets to separate the signal from noise. -- The timing information might be inaccurate in development mode since this slows things down a ton. However, it's still useful for seeing which things take the longest and it's not too difficult to look and see which things are re-rendering. - -**Suggested:** [React Performance Profiling](https://calibreapp.com/blog/react-performance-profiling-optimization) - -### Hermes Profiling (Android only) - -It's possible, but slightly trickier to profile the JS running on Android devices as it does not run in a browser but a JS VM that React Native must spin up first then run the app code. The VM we are currently using on both Android and iOS is called [Hermes](https://reactnative.dev/docs/profile-hermes) and is developed by Facebook. - -In order to profile with Hermes follow these steps: - -- In the metro bundler window press `d` on your keyboard to bring up the developer menu on your device. -- Select "Settings" -- Select "Start Sampling Profiler on Init" -- In metro bundler refresh by pressing r -- The app will start up and a profile will begin -- Once the app loads take whatever action you want to profile -- Press `d` again and select "Disable Sampling Profiler" -- A toast should appear with a path to a profile -- We need to then convert this into something Chrome Dev Tools can use by typing into terminal `react-native profile-hermes .` -- This should create a json file in the directory where we typed the previous command that we can load up into Chrome Dev Tools "Performance" tab via the "Load Profile" option and inspect further. - -### React DevTools Profiler -- The React DevTools Profiler can also be used to detect similar information to Chrome Dev Tools, but is a little more streamlined. There is also an options cog where you can filter events by cutting at a specified millisecond (length it took for the thing to happen) -- Try checking the option to "Record why each component rendered while profiling". This may provide insights into why the component rendered unnecessarily. - -**Suggested:** [Deep Dive with the React DevTools creator](https://www.youtube.com/watch?v=nySib7ipZdk) - -### Why Did You Render? -- Why Did You Render (WDYR) sends console notifications about potentially avoidable component re-renders. -- It can also help to simply track when and why a certain component re-renders. -- To enable it, set `USE_WDYR=true` in your `.env` file. -- You can add or exclude tracked components by their `displayName` in `wdyr.js`. -- Open the browser console to see WDYR notifications. - -**Suggested** [Why Did You Render docs](https://github.com/welldone-software/why-did-you-render) - -### Performance Metrics (Opt-In on local release builds) - -To capture reliable performance metrics for native app launch we must test against a release build. To make this easier for everyone to do we created an opt-in tool (using [`react-native-performance`](https://github.com/oblador/react-native-performance) that will capture metrics and display them in an alert once the app becomes interactive. To set this up just set `CAPTURE_METRICS=true` in your `.env` file then create a release build on iOS or Android. The metrics this tool shows are as follows: - -- `nativeLaunch` - Total time for the native process to intialize -- `runJSBundle` - Total time to parse and execute the JS bundle -- `timeToInteractive` - Rough TTI (Time to Interactive). Includes native init time + sidebar UI partially loaded - -#### How to create a Release Build on Android - -- Create a keystore by running `keytool -genkey -v -keystore your_key_name.keystore -alias your_key_alias -keyalg RSA -keysize 2048 -validity 10000` -- Fill out all the prompts with any info and give it a password -- Drag the generated keystore to `/android/app` -- Hardcode the values to the gradle config like so: - -``` -signingConfigs { - release { - storeFile file('your_key_name.keystore') - storePassword 'Password1' - keyAlias 'your_key_alias' - keyPassword 'Password1' - } -``` -- Delete any existing apps off emulator or device -- Run `react-native run-android --variant release` - -## Reconciliation - -React is pretty smart and in many cases is able to tell if something needs to update. The process by which React goes about updating the "tree" or view heirarchy is called reconciliation. If React thinks something needs to update it will render it again. React also assumes that if a parent component rendered then it's child should also re-render. - -Re-rendering can be expensive at times and when dealing with nested props or state React may render when it doesn't need to which can be wasteful. A good example of this is a component that is being passed an object as a prop. Let's say the component only requires one or two properties from that object in order to build it's view, but doesn't care about some others. React will still re-render that component even if nothing it cares about has changed. Most of the time this is fine since reconciliation is pretty fast. But we might run into performance issues when re-rendering massive lists. - -In this example, the most preferable solution would be to **only pass the properties that the object needs to know about** to the component in the first place. - -Another option would be to use `shouldComponentUpdate` or `React.memo()` to add more specific rules comparing `props` to **explicitly tell React not to perform a re-render**. - -React might still take some time to re-render a component when it's parent component renders. If it takes a long time to re-render the child even though we have no props changing then we can use `PureComponent` or `React.memo()` (without a callback) which will "shallow compare" the `props` to see if a component should re-render. - -If you aren't sure what exactly is changing about some deeply nested object prop you can use `Performance.diffObject()` method in `componentDidUpdate()` which should show you exactly what is changing from one update to the next. - -**Suggested:** [React Docs - Reconciliation](https://reactjs.org/docs/reconciliation.html) +This file has been moved [here](./docs/PERFORMANCE.md). diff --git a/PR_REVIEW_GUIDELINES.md b/PR_REVIEW_GUIDELINES.md index 0470e7ceb31a..fd73c3864ba1 100644 --- a/PR_REVIEW_GUIDELINES.md +++ b/PR_REVIEW_GUIDELINES.md @@ -1,72 +1 @@ -# PR review guidelines - -This document lists specific guidelines for all PR reviews and aims to clarify items in the PR template checklist. - -# Check PR template - -1. Make sure the “Fixed Issues” section is formatted correctly. Links should be clickable and lead to the correct issue being fixed by this PR. -2. Make sure the testing steps are clear and correctly test the changes made. Make note of what’s required for the test account (i.e. do you need to test using a brand new account, have a workspace, have a bank account, etc). - - > **Rule of thumb**: Over-explain what the tester needs to do. Do not rely on the tester having existing knowledge of the app. - > - > ### Example of bad steps: - > - > 1. Go to this page - > 2. Click the button - > - > ### Example of good steps: - > - > 1. Log in with an account that has a workspace, or follow `[these steps](link-to-instructions-for setting-up-workspace)` to make one. - > 2. Click the settings button on the top left (the one with your avatar) - > 3. Navigate to “Your Workspace” > Reimburse Receipts > Connect bank account - > 4. Verify that the bank account view opens without errors - > - - Take note of the distinction between testing _locally_ and _on staging_. Note: The staging site references the production version of the API. -3. Make sure that the test includes both **success** and **fail** scenarios. - - Ex: if the issue was to prevent the user from saving a field when it is empty, besides testing that we should also test that they can save the field _if it is not empty_. - -# Testing the changes - -1. Make sure there are no console errors related to the PR. If you find console errors in a branch you’re reviewing, check if they also exist in `main`. - 1. If they do, proceed as normal (report them in #expensify-open-source if you think they need to be fixed). - 2. If the errors do not exist in `main`, report it to the PR author - they need to be fixed before the PR can be merged. -2. Test the changes on **all platforms**. Follow this guide (needs link) to set up each platform. - - If you’re unable to boot a platform for any reason, ask for help in the #expensify-open-source Slack channel. Your issue might already have been addressed there, so be sure to search first. - -# Reviewing the code - -## Good code patterns to require - -1. Check that functions have comments when appropriate. - - If the function has params or returns something, use [JSDocs syntax]((https://github.com/Expensify/App/blob/main/STYLE.md#jsdocs)) to describe them. - - Indicate the param name(s), data type passed and / or returned, and purpose if not immediately obvious. - - Obvious functions (with no params / return value) should not have comments. - - **In short: _Add comments & docs, only when useful._** -2. All copy / text shown to users in the product should be stored in the `src/languages/*` files for localization. -3. Platform-specific files should follow the proper naming convention (see [Platform-Specific File Extensions](https://github.com/expensify/app#platform-specific-file-extensions)) - - Example: `MyComponent/index.android.js` - -## Code patterns to improve - -1. Platform-specific solutions should not be applied to more platforms than necessary - - E.g. solutions for Android or iOS should go in specific `index.android.js` or `index.ios.js` files, not in an `index.native.js` file. -2. Were any new styles added? Are we following the guidelines in [`STYLING.md`](./STYLING.md)? - -## Code patterns to avoid - -1. Make sure any `setTimeout`s are absolutely necessary, not just a workaround for a situation that isn’t fully understood (e.g. a race condition). -2. Ensure Onyx is used appropriately. ([docs](https://github.com/expensify/react-native-onyx#merging-data)) - -## Considerations around code reuse - -While trying to keep our code DRY (don't repeat yourself), you may find yourself trying to decide between refactoring & reusing a component vs. creating a brand new component. When in doubt about whether something should be reused or refactored, ask for help. - -Here are some guidelines we use in order to make the best decisions: - -1. Specialization - - When one component is a special case of another, we should opt for the technique of composition. Here are the [React docs](https://reactjs.org/docs/composition-vs-inheritance.html#specialization) on Specialization. The idea is that it’s better to establish a pattern of creating increasingly specific components, instead of adding dozens of use cases to a single component. - - You might consider this when a component is being reused because the new use case is “close enough” to the original. Rather than adding functionality to that component with subtle additions, it may be better to create a new, more specialized version of that component. -1. Refactors - - During a code review it often becomes apparent that a refactor is needed. In this case, we recommend following these steps: - 1. Identify the refactor and confirm with others that it’s needed. - 2. Do that refactor first (as a separate job, if it qualifies), merge it, test for regressions. - 3. Proceed with the original issue & job. +This file has been moved [here](./docs/PR_REVIEW_GUIDELINES.md). diff --git a/README.md b/README.md index 45f2cc90f46a..198a9cd7eed4 100644 --- a/README.md +++ b/README.md @@ -20,10 +20,10 @@ #### Additional Reading * [API Details](docs/API.md) -* [Offline First](./OfflineUX.md) -* [Contributing to Expensify](./CONTRIBUTING.md) -* [Expensify Code of Conduct](./CODE_OF_CONDUCT.md) -* [Contributor License Agreement](./CLA.md) +* [Offline First](./docs/OFFLINE_UX.md) +* [Contributing to Expensify](./docs/CONTRIBUTING.md) +* [Expensify Code of Conduct](./docs/CODE_OF_CONDUCT.md) +* [Contributor License Agreement](./docs/CLA.md) ---- @@ -86,7 +86,7 @@ variables referenced here get updated since your local `.env` file is ignored. requests to the backend. External contributors should set this to `true` otherwise they'll have CORS errors. If you don't want to start the proxy server set this explicitly to `false` - `CAPTURE_METRICS` (optional) - Set this to `true` to capture performance metrics and see them in Flipper - see [PERFORMANCE.md](PERFORMANCE.md#performance-metrics-opt-in-on-local-release-builds) for more information + see [PERFORMANCE.md](./docs/PERFORMANCE.md#performance-metrics-opt-in-on-local-release-builds) for more information - `ONYX_METRICS` (optional) - Set this to `true` to capture even more performance metrics and see them in Flipper see [React-Native-Onyx#benchmarks](https://github.com/Expensify/react-native-onyx#benchmarks) for more information @@ -286,9 +286,9 @@ This application is built with the following principles. 4. Brain pushes data into UI inputs (Device input -> React component). 5. UI inputs push data to the server (React component -> Action -> XHR to server). 6. Go to 1 - ![New Expensify Data Flow Chart](/web/data_flow.png) + ![New Expensify Data Flow Chart](/docs/data_flow.png) 1. **Offline first** - - Be sure to read [OfflineFirst.md](./OfflineUX.md)! + - Be sure to read [OFFLINE_UX.md](./docs/OFFLINE_UX.md)! - All data that is brought into the app and is necessary to display the app when offline should be stored on disk in persistent storage (eg. localStorage on browser platforms). [AsyncStorage](https://reactnative.dev/docs/asyncstorage) is a cross-platform abstraction layer that is used to access persistent storage. - All data that is displayed, comes from persistent storage. 1. **UI Binds to data on disk** diff --git a/STORYBOOK.md b/STORYBOOK.md index cb4cf290a77a..9f4a3c1f6c1c 100644 --- a/STORYBOOK.md +++ b/STORYBOOK.md @@ -1,71 +1 @@ -## Storybook - ->Storybook is an open source tool for building UI components and pages in isolation. It streamlines UI development, testing, and documentation. - -At Expensify, we primarily use [Storybook](https://storybook.js.org/) to provide interactive documentation of our design system. This helps bridge the gap between design and engineering to encourage code reusibility and improve communication. - -### Building and Deploying Storybook - -The Storybook docs deploy automatically so there's nothing extra to do here. Storybook is built to the `/dist` folder and lives at [`https://new.expensify.com/docs/index.html`](https://new.expensify.com/docs/index.html). - -To test a local build we can run - -``` -npm run storybook-build -``` - -This will create a `/docs` directory in `/dist` with the static site. - -### Local Testing - -To skip building and load Storybook on a local server run: - -``` -npm run storybook -``` - -### What's a "story" and how do I create one? - ->A story captures the rendered state of a UI component. Developers write multiple stories per component that describe all the “interesting” states a component can support. - -All of our stories are located in a single directory `/src/stories`. - -To create a new story for a component that does not yet have one it's a good idea to copy an existing story file and then update the relevant details. We are using the [Component Story Format](https://storybook.js.org/docs/react/writing-stories/introduction#component-story-format) to write our stories. - -Here's an example story: - -```javascript -import React from 'react'; -import Button from '../components/Button'; - -const story = { - // Title field will determine how the story displays in the sidebar - title: 'Components/Button', - component: Button, -}; - -// Optional `args` are passed which determine the props the example component will have -const Template = args =>