[Fix][Web] App crash on device toolbar toggle

[kacper-mikolajczak]

Details

Fixed Issues

$ #30278
PROPOSAL: #30278 (comment)

Tests

• Verify that no errors appear in the JS console

1. Open devtools
2. If not selected before, change input mode to touch in device toolbar options
3. Toggle device toolbar and check if app crashes

Offline tests

QA Steps

• Verify that no errors appear in the JS console

1. Open devtools
2. If not selected before, change input mode to touch in device toolbar options
3. Toggle device toolbar and check if app crashes

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 / Chrome
  • iOS / native
  • iOS / 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

N/A

Android: mWeb Chrome

N/A

iOS: Native

N/A

iOS: mWeb Safari

N/A

MacOS: Chrome / Safari

Screen.Recording.2023-10-26.at.21.56.28.mov

MacOS: Desktop

Screen.Recording.2023-10-26.at.22.00.20.mov

[melvin-bot]

@ArekChr 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]

[ArekChr]

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

Android: Native

N/A

Android: mWeb Chrome

N/A

iOS: Native

N/A

iOS: mWeb Safari

N/A

MacOS: Chrome / Safari

web.mov

MacOS: Desktop

desktop.mov

[ArekChr]

All works, thanks for PR!

[melvin-bot]

We did not find an internal engineer to review this PR, trying to assign a random engineer to #30278 as well as to this PR... Please reach out for help on Slack if no one gets assigned!

[youssef-lr]

LGTM! Thanks a lot for figuring this out @kacper-mikolajczak!

[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	1027.433 ms → 1166.931 ms (+139.498 ms, +13.6%) 🔴
App start runJsBundle	712.767 ms → 816.783 ms (+104.017 ms, +14.6%) 🔴

Show details

Name	Duration
App start TTI	Baseline Mean: 1027.433 ms Stdev: 25.603 ms (2.5%) Runs: 984.5167879997753 989.8382749999873 990.0254560001194 991.7735000001267 993.8979460000992 995.5300010000356 996.0153899998404 996.6565709998831 998.7327840002254 1001.8813129998744 1002.0825579999946 1002.0915219997987 1002.5616649999283 1003.5853340001777 1003.8776839999482 1004.6078320001252 1007.5807039998472 1012.4025200000033 1013.136737999972 1014.8915289998986 1015.0933909998275 1015.213041999843 1016.1392470002174 1017.0308210002258 1017.4958500000648 1021.2074759998359 1022.0608169999905 1027.1848570001312 1028.177056999877 1028.6960800001398 1028.8407589998096 1029.2230770001188 1030.4701809999533 1030.9272850002162 1032.8848680001684 1033.5982409999706 1033.6257810001262 1033.8842230001464 1037.4606980001554 1038.2117269998416 1039.5046089999378 1039.7531880000606 1040.8451089998707 1041.422009000089 1042.3578809997998 1044.4302210002206 1044.7532049999572 1044.780487000011 1045.7339189997874 1047.1220979997888 1055.4355770000257 1063.5413350001909 1064.2379560000263 1064.8994160001166 1066.0921629997902 1074.1229670001194 1074.47313700011 1079.7076449999586 1102.2253600000404 Current Mean: 1166.931 ms Stdev: 45.971 ms (3.9%) Runs: 1053.4205590002239 1072.206612999551 1097.1982380002737 1102.3052880000323 1103.0002009999007 1108.4515669997782 1118.3555960003287 1118.4771729996428 1122.8870010003448 1125.3596710003912 1130.1551120001823 1130.8102329997346 1130.8141780002043 1133.7331870002672 1135.0063300002366 1135.1144740004092 1135.6188479997218 1136.6299099996686 1140.1679339995608 1140.946108000353 1141.0816360004246 1143.4410469997674 1146.9053410002962 1147.3908590003848 1153.6015250002965 1155.3345480002463 1156.8354860004038 1158.3651219997555 1158.367823000066 1160.3743019998074 1164.0888299997896 1169.4836710002273 1173.8331140000373 1174.0443479996175 1178.082861999981 1179.0391570003703 1179.3603509999812 1180.82928000018 1182.6527169998735 1189.4846750004217 1192.4882559999824 1194.0363140003756 1197.0383700001985 1202.1042499998584 1202.2869149995968 1203.7026899997145 1204.0986900003627 1208.728414000012 1212.1097870003432 1214.1450929995626 1219.1222000000998 1220.4964770004153 1223.4545269999653 1225.355879000388 1240.762350000441 1247.066855000332 1254.9718770002946 1257.0773419998586 1266.6463409997523
App start runJsBundle	Baseline Mean: 712.767 ms Stdev: 20.746 ms (2.9%) Runs: 663 668 684 687 689 691 691 692 693 694 694 694 695 695 695 695 697 697 698 699 701 701 701 702 702 704 705 706 707 707 711 712 714 716 718 719 721 723 725 726 727 727 728 728 728 730 730 732 733 736 737 738 739 739 740 742 744 749 753 754 Current Mean: 816.783 ms Stdev: 40.867 ms (5.0%) Runs: 712 732 750 761 762 767 769 774 775 777 779 780 781 782 784 793 793 794 797 797 798 800 800 801 804 806 807 807 808 815 816 817 819 820 822 824 826 828 829 830 831 838 845 848 849 851 851 860 861 862 866 867 869 869 870 871 879 879 894 911

Meaningless Changes To Duration

Show entries

Name	Duration
Open Search Page TTI	703.770 ms → 720.647 ms (+16.876 ms, +2.4%)
App start regularAppStart	0.013 ms → 0.014 ms (+0.001 ms, +5.4%)
App start nativeLaunch	21.926 ms → 20.357 ms (-1.569 ms, -7.2%)

Show details

Name	Duration
Open Search Page TTI	Baseline Mean: 703.770 ms Stdev: 35.848 ms (5.1%) Runs: 636.3581949998625 647.465494999662 649.8473310000263 653.0703130001202 657.7374269999564 661.5447180001065 662.3345949999057 662.6969000003301 662.9300939999521 663.600424000062 664.4754650001414 664.9874680000357 665.5349540002644 670.0622560000047 670.3770350003615 673.0072840000503 677.4506429997273 681.4790039998479 682.9186609997414 683.8804119997658 686.3913579997607 689.3111570002511 689.5723069999367 690.43802900007 690.6860360000283 690.9610600001179 691.2552900002338 691.5508630000986 698.0932620000094 701.516601999756 705.2502859998494 706.2248939997517 707.3380130003206 710.606201000046 711.4665120001882 713.0528570003808 713.4774170001037 714.9016929999925 715.3793130000122 715.465576000046 715.5013839998282 722.1066080001183 723.7006439999677 725.7929689995944 727.394125000108 728.2416179999709 729.1312669999897 734.5617280001752 736.419474999886 746.2780359997414 746.6089280000888 747.1997890002094 750.2204599999823 754.1767980000004 760.1216639997438 760.497762999963 763.4614260001108 774.1960450001061 776.3072510003112 779.6121430001222 Current Mean: 720.647 ms Stdev: 57.959 ms (8.0%) Runs: 628.7171229999512 635.6590980002657 643.4489749995992 646.509277000092 652.807210999541 652.9451500000432 655.39477599971 659.3114019995555 661.3157959999517 668.0550950001925 669.0198570005596 669.4341229991987 671.7437750007957 672.7438160004094 673.8188889995217 675.6400959994644 676.1141360001639 677.0776370000094 678.1431080000475 680.800904000178 687.968465000391 690.5243330001831 690.5711260000244 691.3906659996137 691.9488119995221 694.9182529998943 696.7583409994841 697.078614000231 704.0028079999611 705.2858480000868 707.5269370004535 708.1132810004056 708.2400309992954 720.553181999363 723.1918950006366 725.3057049997151 729.6897790003568 732.6704510003328 735.4552420005202 738.7798260003328 743.2573239998892 752.4695639992133 752.665527000092 753.9349370002747 755.693237000145 756.5617270004004 759.7052819998935 760.8854569997638 762.959431999363 763.7305509997532 772.831706000492 782.4853929998353 786.2981369998306 789.9710699999705 798.4233000008389 804.8402109993622 823.8450529994443 824.9315599994734 828.7325849998742 876.0397539995611 880.5034590000287
App start regularAppStart	Baseline Mean: 0.013 ms Stdev: 0.001 ms (5.1%) Runs: 0.012125000357627869 0.012165999971330166 0.01228900020942092 0.012329000048339367 0.012614000122994184 0.012614000122994184 0.012653999961912632 0.012654999736696482 0.012695000041276217 0.012736000120639801 0.012776999734342098 0.012817000038921833 0.012817999813705683 0.01281800027936697 0.012897999957203865 0.01293900003656745 0.0129399998113513 0.012980000115931034 0.012980000115931034 0.013020999729633331 0.013021000195294619 0.013061999808996916 0.013061999808996916 0.013183000031858683 0.013183000031858683 0.013183999806642532 0.01318400027230382 0.013224000111222267 0.013225000351667404 0.013265000190585852 0.013346999883651733 0.013427999801933765 0.01346899988129735 0.01346899988129735 0.013549000024795532 0.013590999878942966 0.013591000344604254 0.013591000344604254 0.013671000022441149 0.013712999876588583 0.01371300034224987 0.01375299971550703 0.013753999955952168 0.013753999955952168 0.013957000337541103 0.014037999790161848 0.014078999869525433 0.01407900033518672 0.014119999948889017 0.014159999787807465 0.014159999787807465 0.014160000253468752 0.014161000028252602 0.014200999867171049 0.014282000251114368 0.014322999864816666 0.014486000407487154 0.01464799977838993 0.014973999932408333 0.01525900000706315 Current Mean: 0.014 ms Stdev: 0.001 ms (6.4%) Runs: 0.0125730000436306 0.012655000202357769 0.012736000120639801 0.012858999893069267 0.012899000197649002 0.013020999729633331 0.013020999729633331 0.013101999647915363 0.013143000192940235 0.013183999806642532 0.013224000111222267 0.013225000351667404 0.013346000574529171 0.013346000574529171 0.013387000188231468 0.013590999878942966 0.013630999252200127 0.013671999797224998 0.013671999797224998 0.013672000728547573 0.013672000728547573 0.013753000646829605 0.013875000178813934 0.013915999792516232 0.013996999710798264 0.0139979999512434 0.01403799932450056 0.01403799932450056 0.014038000255823135 0.014078999869525433 0.014119000174105167 0.014159999787807465 0.01416000071913004 0.014240999706089497 0.014241999946534634 0.014281999319791794 0.014322999864816666 0.014322999864816666 0.014403999783098698 0.014405000023543835 0.01464799977838993 0.0147299999371171 0.0147299999371171 0.014770000241696835 0.014771000482141972 0.014771000482141972 0.014933000318706036 0.01521800085902214 0.015381000004708767 0.015461999922990799 0.015461999922990799 0.015665999613702297 0.015705999918282032 0.015747999772429466 0.015868999995291233 0.01590999960899353 0.01607299968600273
App start nativeLaunch	Baseline Mean: 21.926 ms Stdev: 2.185 ms (10.0%) Runs: 18 19 19 19 19 19 19 20 20 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 23 23 24 24 24 24 24 24 24 25 26 27 27 28 Current Mean: 20.357 ms Stdev: 1.445 ms (7.1%) Runs: 19 19 19 19 19 19 19 19 19 19 19 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 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 23 23 23 24 25

[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/youssef-lr 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 ✅

[OSBotify]

🚀 Deployed to staging by https://github.com/youssef-lr in version: 1.3.94-0 🚀

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

[OSBotify]

🚀 Deployed to staging by https://github.com/youssef-lr in version: 1.3.94-0 🚀

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

[OSBotify]

🚀 Deployed to production by https://github.com/Beamanator in version: 1.3.94-2 🚀

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