Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent refocusing in options-selector and selection-list #29084

Merged
merged 7 commits into from
Oct 10, 2023

Conversation

s-alves10
Copy link
Contributor

@s-alves10 s-alves10 commented Oct 9, 2023

Details

Fixed Issues

$ #22812
PROPOSAL: #22812 (comment)

Tests

  1. Open ND and log in with any account
  2. Tap FAB and select Send message
  3. Select multiple users by tapping Add to group button
  4. Verify that the style of search input doesn't change
  5. Close right modal and tap FAB and select Request money
  6. Input any amount in manual tab and tap Next
  7. Select multiple users by tapping Split button
  8. Verify that the style of search input doesn't change
  9. Go to settings -> workspace -> members
  10. Check/uncheck members and verify that the style of search input doesn't change
  11. Go to settings -> workspace -> members -> invite
  12. Check/uncheck members and verify that the style of search input doesn't change
  • Verify that no errors appear in the JS console

Offline tests

  1. Open ND and log in with any account
  2. Tap FAB and select Send message
  3. Select multiple users by tapping Add to group button
  4. Verify that the style of search input doesn't change
  5. Close right modal and tap FAB and select Request money
  6. Input any amount in manual tab and tap Next
  7. Select multiple users by tapping Split button
  8. Verify that the style of search input doesn't change
  9. Go to settings -> workspace -> members
  10. Check/uncheck members and verify that the style of search input doesn't change
  11. Go to settings -> workspace -> members -> invite
  12. Check/uncheck members and verify that the style of search input doesn't change

QA Steps

Note: All the tests should be done on ios/android native, web, and desktop platforms. In mobile web platforms, search input gets blurred on selection by intention

  1. Open ND and log in with any account
  2. Tap FAB and select Send message
  3. Select multiple users by tapping Add to group button
  4. Verify that the style of search input doesn't change
  5. Close right modal and tap FAB and select Request money
  6. Input any amount in manual tab and tap Next
  7. Select multiple users by tapping Split button
  8. Verify that the style of search input doesn't change
  9. Go to settings -> workspace -> members
  10. Check/uncheck members and verify that the style of search input doesn't change
  11. Go to settings -> workspace -> members -> invite
  12. Check/uncheck members and verify that the style of search input doesn't change
  • 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 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

Web
22812_mac_chrome.mp4
Mobile Web - Chrome
22812_android_chrome.mp4
Mobile Web - Safari
22812_ios_safari.mp4
Desktop
22812_mac_desktop.mp4
iOS
22812_ios_native.mp4
Android
22812_android_native.mp4

@s-alves10 s-alves10 marked this pull request as ready for review October 9, 2023 12:18
@s-alves10 s-alves10 requested a review from a team as a code owner October 9, 2023 12:18
@melvin-bot melvin-bot bot requested review from situchan and removed request for a team October 9, 2023 12:18
@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

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

@@ -70,6 +70,9 @@ const propTypes = {
/** Whether to remove the lateral padding and align the content with the margins */
shouldDisableRowInnerPadding: PropTypes.bool,

/** Whether to take focus when selecting */
shouldTakeFocus: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the name consistent? So use shouldFocusOnSelectRow and reverse conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can, but it looks weird considering the name shouldFocusOnSelectRow as OptionRow's props

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@situchan

When we should refocus on selecting row for options-selector, the option row should not take focus. I thought this makes sense.
Do you still want to rename the props?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, tricky naming. shoudTakeFocus is fine but no one knows what this does when read this code without any context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe shouldPreventDefaultFocusOnClick though it's long name?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldPreventDefaultFocusOnSelect is comparatively near to what we do in the code. So I would vote for it first

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we still want to have the same props name, I suggest shouldPreventDefaultFocusOnSelectRow

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldPreventDefaultFocusOnSelectRow sounds good to me.
I think also shouldPreventDefaultFocusOnSelectItem will work.
But yes, using same prop through out the tree will be better IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Let me rename the props for BaseOptionSelector, BaseOptionsList, BaseSelectionList, BaseListItem, and OptionRow

cc @situchan

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Let me rename the props for BaseOptionSelector, BaseOptionsList, BaseSelectionList, BaseListItem, and OptionRow

cc @situchan

👍

@situchan
Copy link
Contributor

situchan commented Oct 9, 2023

@s-alves10 let's also fix invite page
Though this should have been covered in #27497

Screen.Recording.2023-10-10.at.12.01.08.AM.mov

@s-alves10
Copy link
Contributor Author

@situchan

Workspace invite page was updated. Videos were updated as well. Please take a look

@situchan
Copy link
Contributor

situchan commented Oct 9, 2023

@s-alves10 please correct QA Steps so that QA team doesn't get confused

  • add invite page case
  • mWeb behavior is different

@s-alves10
Copy link
Contributor Author

@situchan

I'm sorry I forgot to update that. Just updated

@situchan
Copy link
Contributor

situchan commented Oct 9, 2023

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 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
web.mov
Mobile Web - Chrome
mchrome.mov
Mobile Web - Safari
msafari.mov
Desktop
desktop.mov
iOS
ios.mov
Android
android1.mov
android2.mov

Copy link
Contributor

@situchan situchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@MonilBhavsar all yours

@s-alves10
Copy link
Contributor Author

@MonilBhavsar

Will you take a look?

@MonilBhavsar
Copy link
Contributor

Yes, reviewing now.

@@ -231,6 +232,7 @@ function WorkspaceInvitePage(props) {
onConfirm={inviteUser}
showScrollIndicator
showLoadingPlaceholder={!didScreenTransitionEnd || !OptionsListUtils.isPersonalDetailsReady(props.personalDetails)}
shouldFocusOnSelectRow={!Browser.isMobile()}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two props - shouldFocusOnSelectRow and shouldPreventDefaultFocusOnSelect. Can we make it one to make it easy to understand?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MonilBhavsar Can you please check this discussion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Commented there

@s-alves10
Copy link
Contributor Author

@MonilBhavsar

Props name updated. Please check

Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MonilBhavsar MonilBhavsar merged commit 81565ec into Expensify:main Oct 10, 2023
13 checks passed
@OSBotify
Copy link
Contributor

✋ 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 github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Oct 10, 2023
@github-actions
Copy link
Contributor

Performance Comparison Report 📊

Significant Changes To Duration

Name Duration
App start TTI 1224.526 ms → 1280.434 ms (+55.908 ms, +4.6%)
App start runJsBundle 835.056 ms → 889.600 ms (+54.544 ms, +6.5%) 🔴
Show details
Name Duration
App start TTI Baseline
Mean: 1224.526 ms
Stdev: 41.695 ms (3.4%)
Runs: 1109.185913999565 1139.1086210003123 1142.8446019999683 1150.7076979996637 1151.4810990002006 1156.2096119998023 1157.308984999545 1164.778115999885 1172.8468850003555 1173.1904279999435 1174.257511000149 1177.53395000007 1178.68064600043 1179.0643729995936 1186.066026000306 1188.8681939998642 1190.338410999626 1191.0396609995514 1192.1652330001816 1193.757458999753 1194.084478000179 1195.3947850000113 1195.5100290002301 1196.1428340002894 1198.9499019999057 1199.382840000093 1199.491336000152 1199.492883999832 1201.2729320004582 1202.3542649997398 1203.8945760000497 1203.9233689997345 1205.5535340001807 1205.8731779996306 1208.4448220003396 1209.3589690001681 1212.4129060003906 1218.2313440004364 1218.9652089998126 1222.1049130000174 1222.5686799995601 1223.3492860002443 1224.3171990001574 1225.0559489997104 1225.1164269996807 1226.4209669996053 1226.611143000424 1227.3966570002958 1227.748176000081 1227.931691000238 1229.5114139998332 1229.9415030004457 1231.271325999871 1232.3513700002804 1236.7506179995835 1236.9973710002378 1238.305231999606 1239.7413179995492 1240.0354709997773 1242.3909879997373 1243.345828000456 1244.6973000001162 1245.3443560004234 1247.5679869996384 1248.3963660001755 1248.641893999651 1250.6600270001218 1251.0158050004393 1254.481017000042 1254.884572999552 1257.4420600002632 1258.539369000122 1261.469352999702 1262.4136229995638 1263.807826999575 1266.0076569998637 1267.041461000219 1267.115012999624 1269.118425999768 1276.748565999791 1278.8832310000435 1279.2046299995854 1281.1598030002788 1283.187668000348 1285.4696660004556 1289.1583930002525 1295.4529870003462 1296.089165999554 1304.6100629996508 1312.4178710002452 1313.382945000194

Current
Mean: 1280.434 ms
Stdev: 41.694 ms (3.3%)
Runs: 1204.6564389998093 1214.5137769998983 1217.6063510002568 1227.3808439997956 1227.9738509999588 1231.6785359997302 1232.7752639995888 1238.5659250002354 1238.6955409999937 1238.9061169996858 1239.1666839998215 1239.922372999601 1240.1951780002564 1241.185134000145 1242.1014679996297 1243.4927780004218 1245.204219000414 1246.1850889995694 1246.2111569996923 1246.2789709996432 1247.178567999974 1247.6186429997906 1247.6551980003715 1247.7744730003178 1251.7278890004382 1252.7866740003228 1254.599932000041 1254.8236680002883 1256.7089269999415 1258.5588419996202 1258.9549700003117 1259.647365000099 1260.0640900004655 1260.6277329996228 1261.6265190001577 1262.1693540001288 1262.6513430001214 1265.3528110003099 1266.1330509996042 1266.2793250000104 1268.4864849997684 1269.63830900006 1270.2533860001713 1270.612138999626 1270.715983999893 1273.7486960003152 1274.2428989997134 1275.2073590001091 1275.2258860003203 1278.0836509997025 1279.2794019998983 1280.0262409998104 1280.2858589999378 1281.030849000439 1281.4264949997887 1283.0835349997506 1283.271741000004 1286.4496020004153 1287.0190129997209 1288.2492680000141 1290.0560219995677 1291.366291999817 1292.8446030002087 1293.869517000392 1294.4256929997355 1295.804654999636 1296.220900000073 1296.405197000131 1301.4872939996421 1305.2401430001482 1309.7412740001455 1313.0417339997366 1316.8921710001305 1323.3363859998062 1325.0381190003827 1325.4288710001856 1337.8144739996642 1341.0537620000541 1342.3533159997314 1342.417127000168 1346.6274070004001 1353.4029679996893 1362.1370109999552 1364.4427650002763 1364.8696630001068 1376.8624360002577 1378.56857999973 1387.3860729997978 1387.4981310004368
App start runJsBundle Baseline
Mean: 835.056 ms
Stdev: 32.235 ms (3.9%)
Runs: 752 766 770 772 783 789 789 791 791 793 797 798 799 802 802 804 805 806 809 809 811 812 813 815 819 819 821 822 824 824 824 825 826 826 826 827 829 830 831 832 832 832 833 833 835 837 838 839 841 842 842 842 843 843 843 844 844 845 849 849 850 851 852 852 855 855 855 855 856 857 857 859 862 862 865 865 866 868 875 879 880 881 882 883 888 897 899 910 915

Current
Mean: 889.600 ms
Stdev: 30.684 ms (3.4%)
Runs: 851 852 852 854 856 857 857 857 859 860 860 861 863 864 864 864 865 865 865 865 866 866 867 867 868 868 868 869 870 870 871 871 871 871 872 872 872 873 874 874 875 875 876 877 878 878 879 880 881 886 886 887 888 889 890 891 891 892 892 893 899 901 903 904 905 905 905 907 908 910 910 912 912 912 912 914 917 922 923 931 944 947 952 956 956 958 962 965 967 970

Meaningless Changes To Duration

Show entries
Name Duration
Open Search Page TTI 619.150 ms → 627.937 ms (+8.788 ms, +1.4%)
App start nativeLaunch 20.904 ms → 20.918 ms (+0.014 ms, ±0.0%)
App start regularAppStart 0.015 ms → 0.015 ms (+0.001 ms, +3.4%)
Show details
Name Duration
Open Search Page TTI Baseline
Mean: 619.150 ms
Stdev: 12.412 ms (2.0%)
Runs: 594.3376870006323 597.071981000714 598.9463300006464 603.318726000376 604.1889249999076 604.543782999739 604.5525310002267 604.8945720000193 604.9586589997634 606.3621829999611 606.5622560000047 606.7971609998494 607.0386560000479 607.1259770002216 607.4950769999996 607.5443110000342 607.6946620000526 608.5314939999953 609.4705809997395 609.4796959999949 609.552896999754 609.6944579994306 609.9908850006759 610.8638110002503 611.8155519999564 612.1601569997147 612.9206959996372 613.253539999947 613.5529389996082 613.6557219997048 614.1165370000526 614.2671309998259 614.3609219994396 614.4140219995752 614.5623789997771 615.1130379997194 615.1538500003517 615.1612139996141 615.4112140005454 615.465657999739 615.5106210000813 616.7503660004586 618.9079589992762 619.4553629998118 619.506021999754 619.734864000231 620.0013020001352 620.5456960005686 620.7056479994208 620.7484940001741 620.9383950000629 621.3817950002849 621.6063240002841 621.6595870004967 621.7115479996428 621.9271240001544 621.9933679997921 622.6122240005061 622.7979739997536 622.9955239994451 623.6886390000582 623.8306070007384 623.9489749995992 624.8131919996813 626.4661870002747 626.5453699994832 626.581339999102 628.5430509997532 628.9366460004821 630.1032720003277 630.8039150005206 631.7639569994062 632.9272060003132 633.6611740002409 635.2353520002216 638.15625 639.0795090002939 640.2915849993005 641.4290369991213 641.7473969999701 645.3600679999217 646.250163000077 651.9136159997433 658.6000169999897

Current
Mean: 627.937 ms
Stdev: 17.239 ms (2.7%)
Runs: 599.9854740006849 600.943521999754 605.3207189999521 606.1823319997638 606.5668950006366 606.6236979998648 606.6636969996616 606.8948160000145 607.6304940003902 607.722005000338 607.7241210006177 608.4392909994349 610.0020750006661 610.8920500008389 610.9263100000098 611.5012210002169 612.0579020008445 612.0865080002695 612.5320640001446 613.6385500002652 613.6935229999945 614.3660889994353 614.5380049999803 614.8503010002896 615.1678879996762 615.2515870006755 615.7823489997536 617.0201820004731 617.0215659998357 617.0932210003957 617.1295170001686 617.3098969999701 618.2701830007136 618.3359789997339 618.6996659999713 620.6397310001776 621.0353189995512 621.7070720000193 622.4204099997878 622.9092620005831 623.0088710002601 623.4316409993917 623.8773600002751 624.0385739998892 625.0864260001108 625.7012130003422 626.1066899998114 626.178385999985 629.0086670005694 629.7434080000967 630.0687669999897 630.4289960004389 630.5128589998931 631.0449629994109 631.4294029995799 631.8966479999945 632.4146730005741 632.5848799999803 634.0294190002605 634.12915099971 634.853231000714 635.235596999526 640.2966719996184 640.5275879995897 640.8588460003957 640.9603279996663 641.5344650000334 642.3311360003427 643.5324309999123 644.1676030000672 646.6522220000625 646.6903900001198 647.650512999855 648.2847490003332 648.8355709994212 648.9656170001253 649.3395999995992 649.9983729999512 651.9770099995658 652.343139000237 653.0416669994593 657.010214000009 659.0515949996188 660.0932210003957 668.5454919999465 671.6685800002888 673.8125409996137
App start nativeLaunch Baseline
Mean: 20.904 ms
Stdev: 2.028 ms (9.7%)
Runs: 18 18 18 18 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 20 20 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 21 21 21 21 22 22 22 23 23 23 23 23 23 23 23 24 24 24 24 24 24 25 25 26 27 27

Current
Mean: 20.918 ms
Stdev: 1.632 ms (7.8%)
Runs: 18 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 20 20 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 24 24 24 24 25 25 25 25
App start regularAppStart Baseline
Mean: 0.015 ms
Stdev: 0.001 ms (6.3%)
Runs: 0.012776999734342098 0.01285799965262413 0.013264999724924564 0.013346000574529171 0.013508999720215797 0.013549999333918095 0.013631000183522701 0.013712000101804733 0.013794000260531902 0.013876000419259071 0.013956000097095966 0.013956000097095966 0.013996999710798264 0.014079000800848007 0.014079000800848007 0.014119000174105167 0.01411999948322773 0.01411999948322773 0.014120000414550304 0.014159999787807465 0.0142000000923872 0.014201000332832336 0.014201000332832336 0.014281999319791794 0.014321999624371529 0.014363999478518963 0.014363999478518963 0.014444999396800995 0.014566999860107899 0.014607000164687634 0.01460800040513277 0.014688999392092228 0.0147299999371171 0.0147299999371171 0.014770000241696835 0.014810999855399132 0.014851000159978867 0.014851000159978867 0.014891999773681164 0.014932999387383461 0.014932999387383461 0.014933000318706036 0.014973999932408333 0.015013999305665493 0.01501499954611063 0.015095999464392662 0.015096000395715237 0.015096000395715237 0.015096000395715237 0.015137000009417534 0.015137000009417534 0.015177000313997269 0.015217999927699566 0.015217999927699566 0.015217999927699566 0.0152580002322793 0.0152580002322793 0.015259000472724438 0.015259000472724438 0.015298999845981598 0.015300000086426735 0.015339999459683895 0.015381000004708767 0.015461999922990799 0.015463000163435936 0.015544000081717968 0.015665999613702297 0.015666000545024872 0.015705999918282032 0.015747000463306904 0.015786999836564064 0.0157880000770092 0.015828000381588936 0.015868999995291233 0.0161530002951622 0.016195000149309635 0.016195000149309635 0.016235999763011932 0.016276000067591667 0.016316999681293964 0.016479000449180603 0.01664199959486723 0.016642000526189804 0.01680499967187643 0.01680499967187643 0.017090000212192535 0.017171000130474567

Current
Mean: 0.015 ms
Stdev: 0.001 ms (6.1%)
Runs: 0.013508999720215797 0.013753999955952168 0.0138349998742342 0.013915999792516232 0.014038999564945698 0.014119000174105167 0.014120000414550304 0.014161000028252602 0.014200999401509762 0.014282000251114368 0.014404000714421272 0.01444500032812357 0.014526000246405602 0.014526999555528164 0.01460800040513277 0.01464799977838993 0.01464799977838993 0.014689000323414803 0.014730000868439674 0.014770000241696835 0.014852000400424004 0.014891999773681164 0.0148930000141263 0.0148930000141263 0.014973999932408333 0.014973999932408333 0.014974000863730907 0.015014000236988068 0.01501499954611063 0.015015000477433205 0.015054999850690365 0.015096000395715237 0.015096000395715237 0.015137000009417534 0.015177000313997269 0.015177999623119831 0.015299000777304173 0.015300000086426735 0.015339999459683895 0.015339999459683895 0.01534000039100647 0.01534000039100647 0.01537999976426363 0.015381000004708767 0.015421999618411064 0.015422000549733639 0.015461999922990799 0.015502999536693096 0.015544000081717968 0.015544000081717968 0.01566499937325716 0.015665000304579735 0.015665999613702297 0.015665999613702297 0.015666000545024872 0.015705999918282032 0.015705999918282032 0.01570700015872717 0.015747000463306904 0.0157880000770092 0.015868999995291233 0.015868999995291233 0.015868999995291233 0.016032000072300434 0.016112999990582466 0.016114000231027603 0.016154000535607338 0.016154000535607338 0.016154000535607338 0.016234999522566795 0.016276000067591667 0.0163569999858737 0.016358000226318836 0.016439000144600868 0.016520999372005463 0.01664199959486723 0.0166830001398921 0.016764000058174133 0.016844999976456165 0.016927000135183334 0.016927999444305897 0.016968000680208206 0.017048999667167664 0.017293999902904034 0.017374999821186066 0.018025999888777733 0.018148000352084637

@github-actions
Copy link
Contributor

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 1.3.81-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 1.3.83-0 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DeployBlockerCash This issue or pull request should block deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants