Revert "replace onModalHide with onDismiss for the web"

[bondydaa]

Reverts #15298

See thread https://expensify.slack.com/archives/C01GTK53T8Q/p1677278511242269?thread_ts=1677275385.841379&cid=C01GTK53T8Q but we believe this is the root of all PRs having a performance regression #15298 (comment)

copying all the original info from the PR below so the checklists pass, ignore anything you see below 😅

Details

Fixed Issues

#14848
PROPOSAL: #14848 (comment)

Tests

1. Open a report that has historical messages.
2. Press the edit message icon.
3. Press the emoji icon in the edit composer to open the emoji modal.
4. Exit the emoji model by pressing outside.
5. The edit composer will highlight.

• Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

1. Open a report that has historical messages.
2. Press the edit message icon.
3. Press the emoji icon in the edit composer to open the emoji modal.
4. Exit the emoji model by pressing outside.
5. The edit composer will highlight.

• 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 / 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 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 correct English and 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
  • 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 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 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

Web

web.mp4

Mobile Web - Chrome

android-chrome.mp4

Mobile Web - Safari

ios-safari.mp4

Desktop

desktop.mp4

iOS

ios-app.mp4

Android

android-app.mp4

[MelvinBot]

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

[Luke9389]

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

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

[bondydaa]

and now we 🙏

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

Name	Duration
Open Search Page TTI	602.003 ms → 670.788 ms (+68.785 ms, +11.4%) 🔴

Show details

Name

Duration

Open Search Page TTI

Baseline Mean: 602.003 ms Stdev: 15.908 ms (2.6%) Runs: 577.4180910009891 577.94897400029 578.7864180002362 583.6519779991359 584.8610840011388 587.4610600005835 588.0074869990349 589.5491529982537 590.7583009991795 591.0904549993575 592.1475430000573 596.359659999609 596.8740640003234 598.5958659984171 598.9106040000916 599.5698649995029 599.9516189992428 600.515788000077 606.8300790004432 607.3302819989622 607.8021239992231 609.109294001013 610.3179119993001 615.1753739994019 616.2213959991932 617.0509849991649 618.301880998537 623.2297360002995 625.1231699995697 631.7076830007136 641.4428720008582 Current Mean: 670.788 ms Stdev: 12.946 ms (1.9%) Runs: 644.6648760009557 651.000569999218 651.8235269989818 655.3369950000197 655.859864000231 658.9287919998169 661.8623869996518 664.2943930011243 664.4236249998212 665.1791989989579 666.5874429997057 666.9125980008394 667.4967860002071 667.9304210003465 668.6349279992282 668.9444180000573 670.4588219989091 670.5183920003474 673.6589359994978 674.8397220000625 676.0443110000342 677.2641199994832 680.3145759999752 681.3795170001686 682.5668540000916 684.3103429991752 684.8903399985284 692.2955729998648 694.480956999585 700.7303059995174

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	715.062 ms → 736.891 ms (+21.829 ms, +3.1%)
App start runJsBundle	198.031 ms → 204.656 ms (+6.625 ms, +3.3%)
App start regularAppStart	0.014 ms → 0.017 ms (+0.003 ms, +22.5%) 🟡
App start nativeLaunch	21.313 ms → 20.333 ms (-0.979 ms, -4.6%)

Show details

Name	Duration
App start TTI	Baseline Mean: 715.062 ms Stdev: 30.981 ms (4.3%) Runs: 656.6187520008534 676.941957000643 677.2891399990767 680.2521020006388 680.5575109999627 681.0107799991965 686.4635830000043 687.3010959997773 692.4784120004624 693.8305740002543 701.3714220002294 702.7209270000458 705.7574990000576 707.3526380006224 709.5098149999976 710.1639220006764 710.2341169994324 711.7105039991438 714.5736660007387 719.8735339995474 720.6720480006188 723.6433860007674 726.2953730002046 726.9186860006303 744.5601720008999 751.1491579990834 752.3494930006564 754.853701999411 755.3858850002289 757.8343330007046 780.8920339997858 781.4052659999579 Current Mean: 736.891 ms Stdev: 32.216 ms (4.4%) Runs: 684.5578429996967 688.2958680000156 694.9434970002621 694.9944279994816 699.5683030001819 700.380800999701 701.834403000772 713.7027100007981 714.5708990003914 716.3077889997512 718.6041739992797 719.2917470000684 722.8890760000795 729.2936090007424 733.0669560004026 733.4900509994477 736.6636190004647 736.8609759993851 738.0696499999613 738.0735919997096 744.2522320002317 751.4532479997724 754.344722000882 758.1500959992409 765.6335639990866 771.2615869995207 777.6631129998714 780.1901910007 781.8847979996353 788.4053419996053 791.2315929997712 800.5843339990824
App start runJsBundle	Baseline Mean: 198.031 ms Stdev: 20.889 ms (10.5%) Runs: 167 169 171 173 176 182 184 185 186 186 187 187 188 189 192 193 194 197 198 199 199 200 200 203 209 215 221 229 233 238 241 246 Current Mean: 204.656 ms Stdev: 20.323 ms (9.9%) Runs: 170 172 175 176 180 184 185 189 191 192 194 196 199 201 203 205 206 208 212 213 213 214 215 217 217 219 220 222 224 240 242 255
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (7.3%) Runs: 0.012248000130057335 0.012330001220107079 0.01245100051164627 0.0125730000436306 0.012614000588655472 0.012654000893235207 0.012816999107599258 0.012817000970244408 0.01285799965262413 0.012897999957203865 0.012899000197649002 0.012939998880028725 0.012980001047253609 0.013020999729633331 0.013020999729633331 0.01314299926161766 0.013265000656247139 0.013304999098181725 0.013345999643206596 0.013793999329209328 0.013875000178813934 0.013916000723838806 0.014118999242782593 0.014118999242782593 0.014201000332832336 0.014404000714421272 0.014607999473810196 0.014689000323414803 0.015298999845981598 0.015908999368548393 0.015951000154018402 Current Mean: 0.017 ms Stdev: 0.001 ms (5.3%) Runs: 0.015137000009417534 0.015217998996376991 0.015543999150395393 0.015543999150395393 0.015625 0.015706999227404594 0.015786999836564064 0.015949999913573265 0.015950001776218414 0.016154000535607338 0.016154000535607338 0.01619499921798706 0.016234999522566795 0.016316000372171402 0.01631700061261654 0.016479000449180603 0.016519999131560326 0.016600999981164932 0.016682999208569527 0.016683001071214676 0.016805000603199005 0.017007999122142792 0.017212001606822014 0.017455998808145523 0.017496999353170395 0.017578000202775 0.01794400066137314 0.01794400066137314 0.01810699887573719 0.018635999411344528
App start nativeLaunch	Baseline Mean: 21.313 ms Stdev: 3.264 ms (15.3%) Runs: 18 18 18 18 18 18 18 18 18 19 19 19 19 19 20 20 21 21 21 22 22 22 22 25 25 25 25 25 27 27 27 28 Current Mean: 20.333 ms Stdev: 2.055 ms (10.1%) Runs: 18 18 18 18 18 18 19 19 19 19 19 19 19 20 20 20 20 21 21 21 21 21 21 21 22 22 24 24 24 26

[github-actions]

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

[bondydaa]

dang it! going to revert this then and remove the deploy blocker on this.

[OSBotify]

🚀 Deployed to staging by https://github.com/Luke9389 in version: 1.2.77-0 🚀

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

[OSBotify]

🚀 Deployed to staging by https://github.com/Luke9389 in version: 1.2.77-0 🚀

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

[mvtglobally]

@bondydaa since you reverted #15298. Should we still QA it or just this one?

[OSBotify]

🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.2.77-4 🚀

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