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

added composer focus glitch fix #29073

Merged
merged 9 commits into from
Oct 13, 2023

Conversation

ayazalavi
Copy link
Contributor

Details

In this PR, I have added a method to maintain focus on the input box by passing its input reference and modal status. This prevents the composer from stealing focus away from that input box.

Fixed Issues

  1. $ [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [$500] Web - Chat - Edit message box blink on escape from keyboard shortcut popup #26527
  2. PROPOSAL: [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] [$500] Web - Chat - Edit message box blink on escape from keyboard shortcut popup #26527 (comment)

Tests

  1. Open the app
  2. Open any report
  3. Edit any message
  4. Click CTRL+J to open keyboard shortcut or click on delete icon of some other chat to open delete modal
  5. now close the modal
  6. Edit message box should not glitch as it works currently on ESC from CTRL+K or CTRL+SHIFT+K shortcuts
  • Verify that no errors appear in the JS console

Offline tests

same as online

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 / 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
Untitled.1.mp4
Untitled.2.mp4
Mobile Web - Chrome
Untitled.4.mp4
Mobile Web - Safari
Untitled.3.mp4
Desktop
Untitled.5.mp4
iOS

Should work same as before

Android

Should work same as before

Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
@ayazalavi ayazalavi requested a review from a team as a code owner October 9, 2023 09:46
@melvin-bot melvin-bot bot requested review from narefyev91 and removed request for a team October 9, 2023 09:46
@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

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

Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
@@ -124,6 +129,39 @@ function ReportActionItemMessageEdit(props) {
isFocusedRef.current = isFocused;
}, [isFocused]);

useEffect(() => {
if (Platform.OS === 'web' && !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.

we should not use such checks - you need to create a separate files for handle web and other platforms

if (Platform.OS === 'web' && !Browser.isMobile()) {
InputFocus.composerFocusKeepFocusOn(textInputRef.current, isFocused, modal, onyxFocused);
}
return () => {};
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this is not needed

Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
@narefyev91
Copy link
Contributor

@ayazalavi hey! Seems issue with jumping borders are not fixed on desktop

Screen.Recording.2023-10-09.at.17.52.17.mov
Screen.Recording.2023-10-09.at.17.50.23.mov

Also i think default behaviour now changed - when i open a comment for edit -> open shortcuts modal -> and close it:
Expected behaviour - main composer should be focused
Actual result - edit field is focused

Also when i after it click on main composer -> open shortcuts modal -> and close it:
Expected behaviour - main composer should be focused
Actual result - edit field is focused

Screen.Recording.2023-10-09.at.17.56.40.mov

Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
@ayazalavi ayazalavi force-pushed the ayaz/26527_edit_message_glitch branch from 3035b04 to a127491 Compare October 9, 2023 15:49
@ayazalavi
Copy link
Contributor Author

ayazalavi commented Oct 9, 2023

@narefyev91 I have uploaded index.desktop.ts so it should work fine on desktop as well now.

I am using the same technique on mobile to return focus to the last input field we had focused on. Other than that, the React Native modal also returns focus to the last input it had focus on. So, this is the default behavior as well.

Untitled.9.mp4

Other than that if user explicitly set focus on compose box then focus will stay there. This matches with mobile web as well.

Untitled.10.mp4

@ayazalavi
Copy link
Contributor Author

@narefyev91 have you been able to check PR? any more changes you want?

@narefyev91
Copy link
Contributor

@narefyev91 have you been able to check PR? any more changes you want?

in progress

@narefyev91
Copy link
Contributor

@ayazalavi one think i noticed comparing how staging and local changes are working
When we go back from screen (closing right panel) we should focus main composer - and not any focus edit messages
Staging:

staging.mov

Locally:

locally.mov

@narefyev91
Copy link
Contributor

Hmmm interesting we have some different behaviours between web, mobile web and native
Let's for now leave as it is - i will clarify

@ayazalavi
Copy link
Contributor Author

@narefyev91, yes, all three platforms have different implementations. React Native Modal, by default, puts focus back on the last input that was focused. So, I am trying to maintain that behavior while avoiding the glitch. Secondly, if the edit message is opened, then it makes perfect sense to put the focus back on it once the modal gets closed unless the user explicitly clicks on the main composer box.

If it is possible to merge this PR by tomorrow, then I'll be really grateful to get the bonus.

@narefyev91
Copy link
Contributor

@ayazalavi I tested on web and desktop - looks good
But seems on latest main we have some issues in logic
When user go back from report - and come back - we focus on edit message composer, when we click to open user details - and go back - we focus main composer - to make it consistently we need to focus edit message composer

Screen.Recording.2023-10-11.at.16.25.53.mov

@ayazalavi
Copy link
Contributor Author

@ayazalavi I tested on web and desktop - looks good But seems on latest main we have some issues in logic When user go back from report - and come back - we focus on edit message composer, when we click to open user details - and go back - we focus main composer - to make it consistently we need to focus edit message composer

Screen.Recording.2023-10-11.at.16.25.53.mov

React native modal puts focus back to the last input that was focused before modal was opened. So as soon as user details page closes then basically focus will go back automatically to the edit input box. We cannot put focus back to main composer in this case.

@narefyev91
Copy link
Contributor

narefyev91 commented Oct 11, 2023

@ayazalavi I tested on web and desktop - looks good But seems on latest main we have some issues in logic When user go back from report - and come back - we focus on edit message composer, when we click to open user details - and go back - we focus main composer - to make it consistently we need to focus edit message composer
Screen.Recording.2023-10-11.at.16.25.53.mov

React native modal puts focus back to the last input that was focused before modal was opened. So as soon as user details page closes then basically focus will go back automatically to the edit input box. We cannot put focus back to main composer in this case.

@ayazalavi Yes - it should focus edit composer - but as you can see on the video - it focused main composer when coming back from personal details screen

Signed-off-by: Ayaz Alavi <ayaz.alavi@gmail.com>
@ayazalavi
Copy link
Contributor Author

@ayazalavi I tested on web and desktop - looks good But seems on latest main we have some issues in logic When user go back from report - and come back - we focus on edit message composer, when we click to open user details - and go back - we focus main composer - to make it consistently we need to focus edit message composer
Screen.Recording.2023-10-11.at.16.25.53.mov

React native modal puts focus back to the last input that was focused before modal was opened. So as soon as user details page closes then basically focus will go back automatically to the edit input box. We cannot put focus back to main composer in this case.

@ayazalavi Yes - it should focus edit composer - but as you can see on the video - it focused main composer when coming back from personal details screen

okay got it, I have added changes.

@narefyev91
Copy link
Contributor

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 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
android-web.mov
Mobile Web - Safari
ios-web.mov
Desktop
desktop.mov
iOS
ios.mov
Android
android.mov

Copy link
Contributor

@narefyev91 narefyev91 left a comment

Choose a reason for hiding this comment

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

LGTM! Working fine and resolved the main issue
🎀 👀 🎀 C+ reviewed

@melvin-bot melvin-bot bot requested a review from amyevans October 11, 2023 17:07
@ayazalavi
Copy link
Contributor Author

@amyevans, I hope you're well. Could you please review my pull request as soon as possible? Your prompt feedback would be highly appreciated. Thank you in advance for your quick attention.

@amyevans
Copy link
Contributor

Sorry, it's on my to do list, I just have several higher priority items I need to get through first. Aiming for later today or tomorrow at the latest.

Copy link
Contributor

@amyevans amyevans left a comment

Choose a reason for hiding this comment

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

Looks good, thanks guys!

@amyevans amyevans merged commit 6fefd26 into Expensify:main Oct 13, 2023
14 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 13, 2023
@github-actions
Copy link
Contributor

Performance Comparison Report 📊

Significant Changes To Duration

Name Duration
App start TTI 1276.960 ms → 1692.895 ms (+415.935 ms, +32.6%) 🔴
App start runJsBundle 876.846 ms → 1194.250 ms (+317.404 ms, +36.2%) 🔴🔴
Show details
Name Duration
App start TTI Baseline
Mean: 1276.960 ms
Stdev: 45.786 ms (3.6%)
Runs: 1170.5171419996768 1176.6816299995407 1178.1515809996054 1192.8960239998996 1197.2852640002966 1198.8226410001516 1201.2440780000761 1208.152160000056 1213.4253550004214 1222.8544680001214 1231.2877430003136 1233.5469180000946 1233.9521420001984 1234.5344359995797 1234.6209840001538 1239.1590320002288 1239.5428280001506 1241.5973720001057 1244.4484360003844 1245.2603510003537 1246.3238559998572 1248.2549179997295 1249.642598000355 1250.0542630003765 1253.3024859996513 1253.6235290002078 1256.327188000083 1256.692966000177 1257.2954390002415 1258.8737099999562 1260.447487999685 1260.4508279999718 1261.1581610003486 1261.5845510000363 1264.242920000106 1265.1849980000407 1267.960459000431 1268.1934350002557 1268.6796439997852 1270.1774669997394 1270.2557460004464 1271.3870249995962 1272.283737000078 1273.8362140003592 1273.9112590001896 1275.3587309997529 1277.4633809998631 1279.003492999822 1280.1800790000707 1280.5395569996908 1280.8909740000963 1281.554604000412 1282.9735120004043 1283.2807539999485 1286.5824659997597 1287.6978500001132 1287.7244170000777 1288.3089560000226 1289.5085129998624 1294.251655999571 1294.55178800039 1294.5595199996606 1297.6354000000283 1298.1259759999812 1301.2544489996508 1301.3612360004336 1303.6511409999803 1306.4007719997317 1308.522578000091 1309.602242000401 1310.2079119998962 1314.2690120004117 1318.6023390004411 1321.5810329997912 1322.205604000017 1326.3311280002818 1333.226913000457 1339.6570800002664 1342.7745219999924 1342.8038990003988 1344.5179920000955 1349.2243459997699 1351.4992000004277 1352.8017069995403 1353.1928479997441 1359.3147710002959 1364.3388790003955 1367.3385819997638 1386.4164289999753

Current
Mean: 1692.895 ms
Stdev: 69.149 ms (4.1%)
Runs: 1515.204106000252 1528.122413000092 1543.192466000095 1556.6207280000672 1569.6660380000249 1580.0081860003993 1583.9836889998987 1586.1549030002207 1586.881521999836 1589.0437730001286 1593.9096590001136 1594.7208329997957 1620.6060549998656 1621.0771289998665 1622.2642729999498 1624.6863789996132 1625.934852000326 1639.0364600000903 1639.212120999582 1643.3726500002667 1645.2012820001692 1646.1321639996022 1646.6691429996863 1653.124300999567 1653.6657819999382 1655.8854099996388 1657.7520460002124 1658.8027269998565 1659.5506030004472 1665.7551440000534 1667.966378999874 1669.7737020002678 1670.3127009999007 1673.509982000105 1674.3206909997389 1681.249041000381 1682.677899999544 1685.5564510002732 1686.4349999995902 1688.9843239998445 1691.0228749997914 1692.9327229997143 1694.8394870003685 1694.9305699998513 1695.3757969997823 1696.2171989995986 1697.9264580002055 1700.1636579995975 1701.021693999879 1702.8197140004486 1704.6666649999097 1709.8873450001702 1710.0445959996432 1711.4366539996117 1711.7974810004234 1711.9450679998845 1712.155265999958 1713.4116150001064 1722.1559439999983 1722.6174739999697 1725.6891409996897 1733.5293690003455 1734.1461979998276 1735.574913999997 1738.5549130002037 1738.9506919998676 1739.2883919999003 1742.6118430001661 1746.491175999865 1746.9788189996034 1748.396533999592 1748.9337309999391 1749.2795050004497 1749.9779200004414 1751.8688359996304 1752.9703019997105 1757.0691059995443 1759.0373369995505 1761.4011909998953 1762.5138760004193 1767.4994799997658 1769.1182829998434 1769.2598519995809 1779.699858999811 1782.939929000102 1785.8839360000566 1785.907247000374 1796.0422520004213 1796.1932889996096 1808.2358940001577 1844.8846620004624 1853.0189779996872
App start runJsBundle Baseline
Mean: 876.846 ms
Stdev: 44.167 ms (5.0%)
Runs: 785 792 796 796 797 805 806 808 809 816 817 818 825 827 828 829 838 842 843 844 849 849 850 850 851 851 854 855 855 856 860 860 861 861 865 865 869 870 873 875 875 876 877 877 878 879 879 880 881 882 882 883 883 884 885 888 890 890 891 891 891 895 895 896 896 897 901 904 904 907 909 910 911 912 915 918 920 922 925 928 929 934 939 943 947 953 958 959 972 974 978

Current
Mean: 1194.250 ms
Stdev: 38.371 ms (3.2%)
Runs: 1109 1113 1123 1124 1130 1138 1141 1146 1149 1149 1149 1150 1151 1154 1154 1158 1158 1159 1160 1161 1161 1162 1163 1164 1166 1166 1169 1171 1173 1173 1175 1176 1176 1179 1181 1182 1183 1186 1187 1187 1187 1188 1188 1191 1191 1192 1192 1192 1195 1196 1199 1201 1201 1202 1208 1210 1211 1211 1212 1212 1212 1214 1215 1216 1216 1217 1218 1219 1222 1222 1223 1223 1224 1225 1226 1227 1227 1227 1234 1238 1239 1240 1242 1244 1248 1250 1256 1259 1260 1277 1283 1293

Meaningless Changes To Duration

Show entries
Name Duration
Open Search Page TTI 633.228 ms → 641.886 ms (+8.658 ms, +1.4%)
App start nativeLaunch 22.264 ms → 23.912 ms (+1.648 ms, +7.4%)
App start regularAppStart 0.015 ms → 0.017 ms (+0.002 ms, +11.4%)
Show details
Name Duration
Open Search Page TTI Baseline
Mean: 633.228 ms
Stdev: 20.912 ms (3.3%)
Runs: 599.1175130000338 599.3077800003812 601.4395350003615 601.9568689996377 602.4783939998597 604.8998619997874 607.7572020003572 609.2097169999033 609.9463299997151 610.0664059994742 611.0055350000039 611.6359869996086 613.5387779995799 614.581828000024 614.7989090001211 614.890625 614.9890140006319 615.388021000661 615.6634929999709 615.8467619996518 616.3932700008154 616.6599130006507 616.6669519999996 616.9305010000244 617.074828999117 617.8017579996958 618.818847999908 619.0836589997634 619.4526370000094 619.5733229992911 620.8019610000774 622.3657639995217 623.0464690001681 623.0800779992715 623.5410970002413 623.8683679997921 624.194174000062 625.2965909997001 625.8912350004539 627.2761639999226 627.324828999117 627.571492000483 628.9873050004244 630.0865890001878 630.9502769997343 631.2878010002896 632.0171720003709 632.1905109994113 633.3747969996184 633.7899580001831 634.0552979996428 634.6560880001634 635.129435999319 635.8057869998738 635.9942230004817 636.081991000101 637.9444590006024 638.9965409999713 640.0640049995854 640.2049160003662 640.5049639996141 640.5956619996578 641.4867759998888 643.5767009994015 644.8176680002362 645.6578370006755 646.2638760004193 648.9059250000864 649.4402259998024 649.762980999425 651.2138679996133 652.5146490000188 655.1358650000766 656.3553470000625 657.453288000077 658.4389649992809 660.9569100001827 661.6919360002503 663.2727049998939 664.5939950002357 665.9894199995324 672.5072840005159 672.5743819996715 674.8610839992762 675.4255779990926 676.1787519995123 682.2296550003812 688.8113210005686

Current
Mean: 641.886 ms
Stdev: 28.680 ms (4.5%)
Runs: 599.3582769995555 599.9850260000676 600.8930260008201 601.0584319997579 608.9567059995607 609.6557210003957 610.472616000101 614.1962489997968 614.5893560005352 614.7172039998695 614.8212890001014 614.8294679997489 614.9230150002986 615.0245370008051 615.1540529998019 615.4699299996719 615.588460999541 616.709472999908 617.0152589995414 617.1399330003187 617.2159019997343 617.6667490005493 618.2621659999713 618.3432209994644 618.881510999985 619.4404710000381 623.5660809995607 623.5717779994011 623.7964679999277 623.9155690008774 624.1742760008201 624.2314050002024 624.6151130003855 624.7318930001929 625.0875660004094 625.5474450001493 625.7196859996766 627.765666000545 630.7192379999906 631.722331000492 632.4178880006075 632.9472260000184 632.9829110000283 633.3623460000381 633.4206539997831 633.8938390007243 634.932252000086 635.5393070001155 636.6811119997874 639.5024420004338 640.2317709997296 641.2685150001198 641.355550000444 641.7701829997823 642.5375169999897 643.696777000092 645.1551919998601 648.6680090008304 648.6825769999996 650.5323890000582 650.8971760002896 651.916300999932 652.0509850000963 652.1120609994978 652.3721529999748 652.4365640003234 656.4453940000385 657.0538739999756 657.3850919995457 658.0874030003324 658.8190930001438 664.5810949997976 664.5941169997677 664.6948250001296 666.9831950003281 668.9657390005887 669.3142100004479 670.8736570002511 674.6094979997724 675.3588870000094 677.7873940002173 684.3707280000672 690.8453780002892 691.7802330004051 693.2728679999709 701.2218029992655 708.4559740005061 709.9399009998888 714.4878749996424 714.9423830006272 715.8208420006558
App start nativeLaunch Baseline
Mean: 22.264 ms
Stdev: 3.334 ms (15.0%)
Runs: 18 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 20 20 21 21 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 22 22 23 23 23 24 24 24 24 24 25 25 25 26 26 26 26 26 27 27 27 27 27 28 28 28 28 28 29 30 32 32

Current
Mean: 23.912 ms
Stdev: 3.373 ms (14.1%)
Runs: 19 19 19 20 20 20 20 20 21 21 21 21 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 23 23 23 23 23 24 24 24 24 24 24 24 24 25 25 25 25 25 25 26 26 26 26 27 27 27 27 27 27 28 28 28 28 28 28 29 29 29 29 30 30 30 32 34 34
App start regularAppStart Baseline
Mean: 0.015 ms
Stdev: 0.001 ms (6.1%)
Runs: 0.013061999343335629 0.013183999806642532 0.01322499942034483 0.013264999724924564 0.013346999883651733 0.013549000024795532 0.013632000423967838 0.013671999797224998 0.013712000101804733 0.01371300034224987 0.01375299971550703 0.01375299971550703 0.013793999329209328 0.013794000260531902 0.013794000260531902 0.013915999792516232 0.013957000337541103 0.013957000337541103 0.014119000174105167 0.014159999787807465 0.014161000028252602 0.014200999401509762 0.014201000332832336 0.014241999946534634 0.014282000251114368 0.014282000251114368 0.014283000491559505 0.014404000714421272 0.01444500032812357 0.01448499970138073 0.014526000246405602 0.014526999555528164 0.014566999860107899 0.014566999860107899 0.01460800040513277 0.01460800040513277 0.014689000323414803 0.014689000323414803 0.014729000627994537 0.0147299999371171 0.014770000241696835 0.014770999550819397 0.014771000482141972 0.014852000400424004 0.014852000400424004 0.0148930000141263 0.014973999932408333 0.014973999932408333 0.015054999850690365 0.015054999850690365 0.015056000091135502 0.015096000395715237 0.015096000395715237 0.015096000395715237 0.015137000009417534 0.015137000009417534 0.015176999382674694 0.015258999541401863 0.015259000472724438 0.015259000472724438 0.015298999845981598 0.015300000086426735 0.01534000039100647 0.01534000039100647 0.015381000004708767 0.015381000004708767 0.015381000004708767 0.015421000309288502 0.015422000549733639 0.015542999841272831 0.015625 0.015705999918282032 0.01574699953198433 0.015828999690711498 0.016073000617325306 0.016193999908864498 0.016195000149309635 0.016316999681293964 0.0163569999858737 0.016358000226318836 0.016519999131560326 0.016560000367462635 0.0167239997535944 0.016805000603199005 0.016845999285578728

Current
Mean: 0.017 ms
Stdev: 0.001 ms (6.6%)
Runs: 0.013631000183522701 0.014689000323414803 0.014852000400424004 0.0148930000141263 0.015096000395715237 0.015137000009417534 0.015217999927699566 0.0152580002322793 0.01534000039100647 0.015381000004708767 0.015461999922990799 0.015463000163435936 0.015502999536693096 0.015502999536693096 0.01550300046801567 0.015625 0.015665999613702297 0.015706000849604607 0.01574699953198433 0.01574699953198433 0.015747000463306904 0.015786999836564064 0.015868999995291233 0.015991000458598137 0.016032000072300434 0.016032000072300434 0.016032000072300434 0.016071999445557594 0.01607200037688017 0.01607299968600273 0.01607299968600273 0.016114000231027603 0.016114000231027603 0.016153999604284763 0.016154000535607338 0.016193999908864498 0.01623500045388937 0.016275999136269093 0.016316000372171402 0.016316000372171402 0.0163569999858737 0.016397999599575996 0.01639800053089857 0.01647899951785803 0.016479999758303165 0.0165200000628829 0.0165200000628829 0.01660200022161007 0.01664199959486723 0.016642999835312366 0.016723000444471836 0.0167239997535944 0.01676399912685156 0.016764000058174133 0.016845999285578728 0.016927000135183334 0.01696799974888563 0.01696799974888563 0.017048999667167664 0.0170499999076128 0.017090000212192535 0.017090000212192535 0.017129999585449696 0.017130999825894833 0.017171000130474567 0.0174150001257658 0.017455999739468098 0.017537999898195267 0.017578000202775 0.017619000747799873 0.01769999973475933 0.017740999348461628 0.017741000279784203 0.017985000275075436 0.018106999807059765 0.018187999725341797 0.0183100001886487 0.018351000733673573 0.018432000651955605 0.019204999320209026 0.019613000564277172 0.019694000482559204

@github-actions
Copy link
Contributor

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

@ayazalavi
Copy link
Contributor Author

@narefyev91 Can we reopen this PR and send update with the performance fix?

@amyevans
Copy link
Contributor

The test is not working accurately right now (it's applying it to all merged PRs at the moment), this is a known issue. I'm going to remove the deploy blocker label.

@amyevans amyevans removed the DeployBlockerCash This issue or pull request should block deployment label Oct 13, 2023
@ayazalavi
Copy link
Contributor Author

Thanks @amyevans for clarifying the issue.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/amyevans in version: 1.3.84-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform result
🤖 android 🤖 skipped 🚫
🖥 desktop 🖥 skipped 🚫
🍎 iOS 🍎 skipped 🚫
🕸 web 🕸 skipped 🚫

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/amyevans in version: 1.3.85-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.85-4 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants