[Form Provider Refactor] PrivateNotesEditPage

[kowczarz]

Details

Fixed Issues

$ #30004

Tests

1. Open a conversation with other user
2. Press their avatar image > Private notes > My note > Notes
3. Type a note
4. Save the note

• Verify that no errors appear in the JS console

Offline tests

Same as above

QA Steps

Same as above

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • 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)
  • 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 the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • 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 is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review 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) 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
• 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
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

android.native_H.265.mp4

Android: mWeb Chrome

android.web_H.265.mp4

iOS: Native

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2023-10-24.at.15.45.43_H.265.mp4

iOS: mWeb Safari

ios.web_H.265.mp4

MacOS: Chrome / Safari

web_H.265.mp4

MacOS: Desktop

desktop_H.265.mp4

[melvin-bot]

@robertKozik Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[robertKozik]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• I verified testing steps are clear and they cover the changes made in this PR
  • I verified the steps for local testing are in the Tests section
  • 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
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see 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 the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • 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 is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review 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) 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
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• 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:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Screen.Recording.2023-10-25.at.13.16.01.mov

Mobile Web - Chrome

android.-.web.mov

Mobile Web - Safari

ios.-.web.mov

Desktop

desktop.mov

iOS

ios.-.native.mov

Android

android.-.native.mov

[robertKozik]

LGTM 🚀

[kowczarz]

Cc. @luacmartins

[OSBotify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

Name	Duration
App start TTI	1190.258 ms → 1250.683 ms (+60.425 ms, +5.1%) 🔴

Show details

Name

Duration

App start TTI

Baseline Mean: 1190.258 ms Stdev: 50.547 ms (4.2%) Runs: 1061.7393110000994 1100.2452529999427 1114.9189339999575 1116.14267500001 1121.1677910001017 1127.2719260000158 1135.0756349998992 1139.2748829999473 1142.9738229999784 1144.8585689999163 1150.0741779999807 1153.53315500007 1156.8090719999745 1157.1457340000197 1159.18207599991 1159.549122999888 1159.898167999927 1160.4197629999835 1161.3659759999719 1162.3037590000313 1164.2155279999133 1164.91642699996 1166.439337000018 1169.3220659999643 1169.3240829999559 1170.5768649999518 1171.688426000066 1172.5280659999698 1173.4651959999464 1176.5352630000561 1180.8939749998972 1181.6255399999209 1183.5754160000943 1184.3343559999485 1191.7073099999689 1200.6111940001138 1201.7962030000053 1211.4404490001034 1212.182152000023 1212.5261770000216 1216.828167000087 1222.821629999904 1226.8010249999352 1227.6389369999524 1227.8945530001074 1227.897280999925 1230.7837169999257 1231.5723000001162 1232.0991229999345 1233.971513000084 1243.3964009999763 1244.7763630000409 1252.1076450000983 1254.2140299999155 1255.2570569999516 1267.9544790000655 1278.6054670000449 1281.6506739999168 1304.2912149999756 1311.2839939999394 Current Mean: 1250.683 ms Stdev: 36.696 ms (2.9%) Runs: 1146.931573000038 1166.3393340000184 1187.2986240000464 1192.8208709999453 1203.1648509999504 1203.8887779999059 1204.0951079999795 1205.4712330000475 1208.4982280000113 1209.8757899999619 1210.6264410000294 1210.9339300000574 1211.0357000000076 1215.811762999976 1219.7276579999598 1223.591872000019 1230.7123660000507 1231.137442999985 1233.2523489999585 1234.5047639999539 1234.7807040000334 1235.7215829999186 1241.0594790000468 1243.1825340000214 1245.2912950000027 1245.5601719999686 1246.0404710000148 1252.6433439999819 1255.236595000024 1258.983773999964 1259.0226659999462 1262.4211900000228 1262.7527719999198 1262.7797040001024 1264.2659370000474 1264.5311879999936 1268.2366260000272 1271.8626520000398 1272.5400370000862 1273.0360629999777 1273.7033059999812 1275.5321100000292 1276.396306999959 1278.5534300000872 1281.2668079999276 1282.0970360001083 1283.7054810000118 1283.8691679999465 1284.3115519999992 1285.26426299999 1287.672964999918 1287.8836469999515 1289.9950830000453 1291.2221099999733 1294.4166470000055 1294.6508780000731 1299.4886010000482 1299.5912220000755 1300.9276209999807 1320.7889419998974

Meaningless Changes To Duration

Show entries

Name	Duration
App start runJsBundle	830.467 ms → 868.817 ms (+38.350 ms, +4.6%)
App start nativeLaunch	20.815 ms → 23.367 ms (+2.552 ms, +12.3%)
Open Search Page TTI	715.112 ms → 715.454 ms (+0.342 ms, ±0.0%)
App start regularAppStart	0.014 ms → 0.015 ms (+0.001 ms, +7.8%)

Show details

Name	Duration
App start runJsBundle	Baseline Mean: 830.467 ms Stdev: 36.428 ms (4.4%) Runs: 742 746 764 769 779 780 784 794 797 797 799 803 805 806 806 808 809 814 814 814 816 816 817 819 820 821 821 822 824 824 828 830 831 836 836 836 838 839 842 844 845 845 851 861 862 863 863 869 873 873 873 877 880 881 881 882 884 885 888 902 Current Mean: 868.817 ms Stdev: 31.241 ms (3.6%) Runs: 779 819 824 830 834 834 835 835 837 837 839 841 842 843 843 843 844 847 849 850 851 853 854 859 859 861 861 862 864 865 867 870 876 877 880 880 881 882 882 883 883 884 884 884 887 888 888 888 893 894 910 910 910 911 912 914 918 926 929 944
App start nativeLaunch	Baseline Mean: 20.815 ms Stdev: 1.656 ms (8.0%) Runs: 18 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 23 23 23 24 24 25 25 25 Current Mean: 23.367 ms Stdev: 3.425 ms (14.7%) Runs: 19 19 19 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 22 22 22 22 22 22 22 22 23 23 24 24 24 24 24 24 24 25 25 25 25 25 26 26 26 26 27 27 29 29 29 29 29 30 30 31 31
Open Search Page TTI	Baseline Mean: 715.112 ms Stdev: 53.430 ms (7.5%) Runs: 640.7265220000409 645.1490889997222 645.4490159999114 646.9820159999654 649.4762369999662 650.394328000024 651.0755620000418 651.3443200001493 654.3870040001348 654.6414799999911 655.2011319999583 655.2699789996259 656.4590659998357 656.5665690000169 664.8413900001906 667.297322999686 669.815837000031 671.8114829999395 672.7869059997611 676.9600430000573 678.0021569998935 681.1829830002971 687.7357180002145 692.203735999763 695.2595629999414 702.5456139999442 705.4425860000774 706.5579840000719 709.2967539997771 709.4777430002578 712.3289390001446 715.7733559999615 720.500326000154 721.3917650002986 725.0141600002535 726.1343590000179 726.1733810002916 727.0677909995429 727.7748620000202 731.1078289998695 732.224040000001 733.3570150001906 734.7206220002845 742.6101080002263 746.6408689999953 747.9262699997053 756.0077720000409 758.7781989998184 760.9694010000676 762.2424320001155 771.5430100001395 771.8794359997846 774.0721839996986 783.821411000099 787.9402270000428 803.5887050000019 808.2014569998719 814.4924719999544 818.3197840000503 834.6367600001395 840.262451000046 Current Mean: 715.454 ms Stdev: 36.913 ms (5.2%) Runs: 626.553549000062 660.9805919998325 661.5361330001615 662.3197429999709 665.0391440000385 666.0021979999729 671.6156010001432 672.530965999933 673.9160569999367 678.1588949998841 679.4704589999747 680.2349859999958 682.6102299999911 684.882690000115 685.4750570000615 686.5335699999705 689.618652000092 690.7480880001094 691.2701419999357 691.3022060000803 692.6868490001652 692.9669599998742 698.5772309999447 699.5098470000084 699.949585000053 706.5373129998334 709.1355800000019 716.847453000024 718.2717699999921 719.293214000063 720.8054609999526 721.5636809999123 723.5387379999738 724.5372729999945 724.8588870000094 725.5764979999512 729.4324550000019 729.9771320000291 731.7164309998043 732.7386469999328 735.5720620001666 736.0574539999943 736.8291830001399 738.1448969999328 738.2349449999165 738.6424569999799 739.0639250001404 739.4388840000611 739.7631020001136 742.5420739999972 743.7376710001845 755.6735029998235 756.5559090001043 762.8935950000305 763.8132740000729 766.5853679999709 782.4303800000343 794.6000169999897 796.7976079999935 800.5651050000452
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (4.8%) Runs: 0.012654999969527125 0.012735999887809157 0.012735999887809157 0.012980000115931034 0.013061999808996916 0.01310200011357665 0.01310200011357665 0.013183000031858683 0.013184000039473176 0.013223999878391623 0.0133050000295043 0.013306000037118793 0.013428000034764409 0.0134680001065135 0.013469000114127994 0.013469000114127994 0.013508999953046441 0.013508999953046441 0.013630999950692058 0.013672000030055642 0.013713000109419227 0.01383400009945035 0.01387499994598329 0.013875999953597784 0.013916000025346875 0.013956999871879816 0.01395700010471046 0.013996999943628907 0.014038000022992492 0.014038000022992492 0.014078999869525433 0.014118999941274524 0.014119999948889017 0.014160000020638108 0.014160000020638108 0.014201000100001693 0.01424099993892014 0.014241999946534634 0.014282000018283725 0.01432300009764731 0.01432300009764731 0.01432300009764731 0.01436399994418025 0.014404000015929341 0.014404000015929341 0.014405000023543835 0.014445000095292926 0.014608000172302127 0.014648000011220574 0.014689000090584159 0.014728999929502606 0.0147299999371171 0.014851999934762716 0.014892000006511807 0.01509599993005395 0.015381000004708767 0.015462999930605292 0.015543999848887324 Current Mean: 0.015 ms Stdev: 0.001 ms (5.3%) Runs: 0.013590999995358288 0.01367199991364032 0.01387499994598329 0.013996999943628907 0.01403799990657717 0.014038000022992492 0.014078999985940754 0.014119000057689846 0.01412000018171966 0.014160000020638108 0.0142000000923872 0.014281999901868403 0.014282000018283725 0.014322999981231987 0.014444999978877604 0.014568000100553036 0.014607999939471483 0.014688999974168837 0.014688999974168837 0.014728999929502606 0.014730000053532422 0.014771000016480684 0.014851999934762716 0.014933999977074564 0.014973999932408333 0.014973999932408333 0.015015000011771917 0.015015000011771917 0.015015000011771917 0.01509599993005395 0.01521800016053021 0.01521800016053021 0.01521899993531406 0.01525900000706315 0.015340000041760504 0.015340000041760504 0.015381000004708767 0.015461999922990799 0.01546200003940612 0.015543000074103475 0.015625 0.015625 0.015666000079363585 0.015705999918282032 0.015706000034697354 0.015706999925896525 0.0157880000770092 0.0157880000770092 0.01595099992118776 0.01599099999293685 0.01603099994827062 0.016073000035248697 0.01607300015166402 0.01615400006994605 0.016316000022925436 0.016805000021122396 0.0172520000487566

[github-actions]

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

[OSBotify]

🚀 Deployed to staging by https://github.com/luacmartins in version: 1.3.92-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by https://github.com/Beamanator in version: 1.3.92-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to staging by https://github.com/luacmartins in version: 1.3.93-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[OSBotify]

🚀 Deployed to production by https://github.com/Beamanator in version: 1.3.93-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅