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

[CP Staging] Revert "Merge pull request #27836 from Expensify/dangrous-roomwelcomemessages" #28955

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

mountiny
Copy link
Contributor

@mountiny mountiny commented Oct 5, 2023

This reverts commit 447d87e, reversing changes made to 7f063c7.

Almost straight revert of #27836

Details

Almost straight revert of the PR which added the textarea and KeyboardAvoidingView

Fixed Issues

$ #28950

Tests

  1. Log into HT account on Expensify app
  2. While having at least 2 workspaces, selecting the FAB button
  3. Select "send message" and select "room" tab
  4. Begin inputting options in any form available on page eg "Who can post", "Visibility", etc
  • Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

Same as tests

  • 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
Screen.Recording.2023-10-05.at.23.35.54.mov
Mobile Web - Chrome
Mobile Web - Safari
Desktop

SAme as web

iOS
RPReplay_Final1696544869.MP4
Android

…messages"

This reverts commit 447d87e, reversing
changes made to 7f063c7.
@mountiny mountiny self-assigned this Oct 5, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Oct 5, 2023

@mountiny mountiny changed the title Revert "Merge pull request #27836 from Expensify/dangrous-roomwelcomemessages" [CP Staging] Revert "Merge pull request #27836 from Expensify/dangrous-roomwelcomemessages" Oct 5, 2023
@mountiny mountiny marked this pull request as ready for review October 5, 2023 22:44
@mountiny mountiny requested a review from a team as a code owner October 5, 2023 22:44
@melvin-bot melvin-bot bot requested review from eVoloshchak and removed request for a team October 5, 2023 22:44
@melvin-bot
Copy link

melvin-bot bot commented Oct 5, 2023

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

@mountiny mountiny merged commit d420b0a into main Oct 5, 2023
14 of 17 checks passed
@mountiny mountiny deleted the vit-revert27836 branch October 5, 2023 22:47
@mountiny
Copy link
Contributor Author

mountiny commented Oct 5, 2023

Straight revert

OSBotify pushed a commit that referenced this pull request Oct 5, 2023
[CP Staging] Revert "Merge pull request #27836 from Expensify/dangrous-roomwelcomemessages"

(cherry picked from commit d420b0a)
@OSBotify
Copy link
Contributor

OSBotify commented Oct 5, 2023

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

@OSBotify
Copy link
Contributor

OSBotify commented Oct 5, 2023

🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.78-4 🚀

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

@github-actions github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Oct 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Name Duration
App start TTI 1278.444 ms → 1370.678 ms (+92.233 ms, +7.2%) 🔴
App start runJsBundle 873.213 ms → 924.386 ms (+51.173 ms, +5.9%) 🔴
Show details
Name Duration
App start TTI Baseline
Mean: 1278.444 ms
Stdev: 43.200 ms (3.4%)
Runs: 1180.7085699997842 1198.4643859993666 1211.1683380007744 1212.6694750003517 1212.9036539997905 1212.9400100000203 1213.006295999512 1214.615568999201 1215.802835000679 1217.1766480002552 1219.754476999864 1226.6059130001813 1231.629205999896 1233.0325169991702 1233.9880950003862 1234.4907649997622 1239.902555000037 1243.241067999974 1245.0805799998343 1245.1887730006129 1245.968382999301 1247.8428199999034 1248.9478459991515 1250.4273649994284 1250.6999290008098 1251.1548529993743 1251.976766999811 1252.6332789994776 1254.5540730003268 1256.5596089996397 1257.4835650008172 1259.690246000886 1260.2087340001017 1261.3839759994298 1263.8300180006772 1265.7656420003623 1268.3707159999758 1269.270480999723 1272.1899209991097 1272.4644399993122 1273.722541000694 1274.489023000002 1276.251528000459 1276.4150060005486 1277.6590480003506 1278.7081959992647 1278.981426000595 1279.7803910002112 1280.5440720003098 1281.9518660008907 1282.433965999633 1282.7412739992142 1285.6226339992136 1286.0660720001906 1288.6963209994137 1289.294517999515 1293.141589000821 1293.283206999302 1294.258602000773 1295.2003680001944 1295.7758920006454 1295.8827880006284 1296.6962059997022 1299.9093759991229 1301.37163499929 1302.6054960004985 1303.0875020008534 1304.8468490000814 1305.1267879996449 1307.0938820000738 1313.6000980008394 1313.6269519999623 1314.088609000668 1315.6911529991776 1320.8929149992764 1322.9282150007784 1323.4798990003765 1326.0916080009192 1327.709578000009 1328.7018790002912 1331.6777069997042 1333.7527920007706 1340.8886200003326 1350.0276640001684 1350.7051529996097 1353.1679650004953 1363.5619539991021 1376.1501749996096 1381.9222999997437 1385.8970759995282

Current
Mean: 1370.678 ms
Stdev: 48.557 ms (3.5%)
Runs: 1295.9652369990945 1301.5614710003138 1304.8900620006025 1306.8669750001281 1307.5792089998722 1310.0946759991348 1310.9759999997914 1312.107208000496 1314.4000039994717 1315.4275080002844 1315.6901450008154 1316.0211849994957 1317.199192000553 1317.7088520005345 1321.8632789999247 1322.2629489991814 1322.7463530004025 1323.2181010004133 1326.2819609995931 1326.4235730003566 1326.6431719996035 1327.9684820007533 1331.1845280006528 1332.3748480007052 1334.9762449990958 1335.3703029993922 1336.548743000254 1336.8092950005084 1338.2303710002452 1341.2024900000542 1341.4579319991171 1342.3543660007417 1347.903030000627 1348.4873900003731 1348.6816140003502 1348.8428360000253 1349.199131000787 1350.8763960003853 1351.2655430007726 1351.6341789998114 1351.7137010004371 1356.791751999408 1357.6799439992756 1358.4506310001016 1362.1045200005174 1363.4700320009142 1363.6909800004214 1366.9439240004867 1371.7637339998037 1372.4892900008708 1372.5936510004103 1374.7253089994192 1374.7549539990723 1375.3175080008805 1376.9063740000129 1377.397569000721 1377.9745760001242 1379.9482039995492 1380.5569550003856 1381.7894289996475 1381.9606539998204 1383.1232880000025 1385.6167329996824 1387.5327950008214 1394.7686550002545 1395.4969800002873 1395.5037539992481 1398.9374320004135 1404.5230020005256 1406.6250560004264 1408.3905829992145 1411.1012239996344 1412.5770319998264 1415.0413969997317 1417.5281659998 1421.6481819991022 1423.1800309997052 1428.1426439993083 1430.6811619997025 1431.7196219991893 1435.8990020006895 1437.827724000439 1441.5719440001994 1443.1344969999045 1444.999502999708 1460.1718210000545 1466.6261349990964 1480.7122409995645 1480.822440000251 1491.2466829996556 1506.2136860005558
App start runJsBundle Baseline
Mean: 873.213 ms
Stdev: 25.173 ms (2.9%)
Runs: 816 818 819 829 829 833 834 835 837 842 842 843 847 851 852 853 854 855 855 855 857 857 857 858 859 859 860 861 862 862 864 864 865 865 865 866 866 867 867 868 868 868 869 870 870 872 872 872 875 877 877 877 877 880 880 880 881 882 882 883 883 884 885 886 889 890 890 891 892 893 898 898 899 901 903 903 905 906 909 910 911 913 913 915 915 916 918 920 920

Current
Mean: 924.386 ms
Stdev: 27.257 ms (2.9%)
Runs: 875 882 882 886 890 891 892 893 894 895 896 897 899 899 899 900 900 901 901 903 904 904 904 905 905 905 905 907 908 908 910 911 911 911 911 913 913 913 914 916 916 917 917 917 918 919 919 920 921 924 925 926 927 930 930 930 934 935 936 938 940 941 942 942 942 942 942 943 944 945 946 947 948 950 951 951 952 952 956 959 959 966 970 975 994 994 996 1005

Meaningless Changes To Duration

Show entries
Name Duration
Open Search Page TTI 644.841 ms → 654.661 ms (+9.820 ms, +1.5%)
App start nativeLaunch 22.356 ms → 24.082 ms (+1.726 ms, +7.7%)
App start regularAppStart 0.016 ms → 0.017 ms (+0.001 ms, +7.6%)
Show details
Name Duration
Open Search Page TTI Baseline
Mean: 644.841 ms
Stdev: 24.379 ms (3.8%)
Runs: 606.7873539999127 609.339925000444 613.9978849999607 616.9934890009463 617.2985849995166 618.1813559997827 619.8169759996235 619.8768319990486 619.9897469989955 620.999552000314 621.0300300009549 621.2726649995893 623.6313889995217 624.0954599995166 624.1744800005108 624.7237550001591 624.7292490005493 625.6441249996424 625.8781339991838 626.394612999633 626.7789309993386 627.0689700003713 627.1653239987791 628.9748129993677 629.8531090002507 629.9593909997493 631.152912998572 632.0290529988706 632.5571699999273 633.0509849991649 633.3579510003328 633.4904380012304 633.8148599993438 634.7199300006032 634.9315189998597 635.4256590008736 635.5824790000916 635.9244800005108 636.0565590001643 636.1341960001737 636.3468019999564 636.9678550008684 637.3369960002601 637.5291750002652 638.1336270011961 638.5993250012398 638.9918209984899 639.0146490000188 639.0454110000283 640.1032720003277 640.6165779996663 640.8232840001583 642.0463870000094 642.2589520011097 642.6555180009454 642.7995199989527 643.5372320003808 644.14131699875 645.2043869998306 645.6988120004535 647.8074960000813 648.3120929989964 649.8533939998597 650.3414309993386 651.0412189997733 652.3636070005596 652.3996169995517 654.5360920000821 654.610555998981 657.2089440003037 658.2347820010036 664.6984050013125 667.0878509990871 667.2252200003713 668.5338139999658 670.0083419997245 670.3080240003765 670.7783209998161 672.8397629987448 685.8457030002028 688.9171139989048 691.8392340000719 695.9193120002747 696.8576260004193 702.5698239989579 703.0539560001343 707.550862999633 712.5808509998024 712.8319089990109

Current
Mean: 654.661 ms
Stdev: 17.415 ms (2.7%)
Runs: 623.922527000308 623.9397780001163 624.0882569998503 625.6560880001634 627.7527270000428 628.9732259996235 630.8002519998699 630.9466549996287 636.0526940003037 637.5503739994019 638.4137369990349 638.4389660004526 638.8223069999367 639.7725829984993 639.8924560006708 640.1378990001976 640.5532629992813 641.171753000468 641.6003830004483 641.9450690001249 642.0683189984411 642.1328940000385 642.3111580014229 642.5185550004244 642.9571529999375 644.1084389984608 644.1938079986721 644.6621910016984 644.9126389995217 644.9581710007042 645.3706059996039 646.3116460014135 647.4328620005399 647.9468180015683 648.2416590005159 648.5111899990588 648.9002279993147 649.2814950011671 649.7551279999316 649.9988209996372 650.0570069998503 650.7958579994738 651.2869879994541 651.6271569989622 651.7898360006511 652.8861490003765 653.3052169997245 653.5752360001206 653.5882569998503 654.3067220002413 655.4205729998648 655.8958339989185 655.9163819998503 656.3992519993335 656.5276289992034 657.0429289992899 658.0542409997433 659.0592849999666 659.2635899987072 661.0087889991701 661.0489499997348 662.3578699994832 662.5050860010087 663.5067139994353 664.2144370004535 664.32397400029 664.4166669994593 664.9708249997348 665.5229489989579 666.2338059991598 667.5238039996475 668.4790039993823 669.715657999739 669.815592000261 672.319947000593 673.9805510006845 674.1590170003474 674.8524580001831 678.5262460000813 679.9515790008008 683.3715010005981 689.4427080005407 689.7670080009848 690.2276619989425 691.2287189997733 691.5959479995072 698.9497070014477 702.3646650016308
App start nativeLaunch Baseline
Mean: 22.356 ms
Stdev: 2.449 ms (11.0%)
Runs: 18 19 19 19 19 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 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 24 24 25 25 25 25 25 25 26 26 26 27 27 27 28 28 28 29 29

Current
Mean: 24.082 ms
Stdev: 2.281 ms (9.5%)
Runs: 20 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 23 24 24 24 24 24 24 24 24 24 24 25 25 25 25 25 25 25 25 25 25 26 26 26 26 26 26 27 27 27 27 27 27 28 28 28 29 29 30 30 30
App start regularAppStart Baseline
Mean: 0.016 ms
Stdev: 0.001 ms (5.5%)
Runs: 0.013672001659870148 0.013793999329209328 0.013996999710798264 0.0139979999512434 0.01428300142288208 0.014445001259446144 0.01464799977838993 0.01464800164103508 0.01476999931037426 0.01476999931037426 0.014852000400424004 0.014852000400424004 0.014893000945448875 0.014932999387383461 0.014933999627828598 0.014972999691963196 0.014973999932408333 0.015015000477433205 0.01505500078201294 0.015055999159812927 0.015096001327037811 0.015135999768972397 0.015137000009417534 0.015137000009417534 0.015137000009417534 0.015177000313997269 0.015178000554442406 0.015300000086426735 0.015300000086426735 0.015380000695586205 0.015421999618411064 0.015502998605370522 0.015543999150395393 0.015543999150395393 0.015584001317620277 0.015625 0.015625 0.015665000304579735 0.015665000304579735 0.015706999227404594 0.015706999227404594 0.01574699953198433 0.0157880000770092 0.015828000381588936 0.015828998759388924 0.015869000926613808 0.01590999960899353 0.01590999960899353 0.01590999960899353 0.01591000147163868 0.015990998595952988 0.015991000458598137 0.01603199914097786 0.01607299968600273 0.01607299968600273 0.01615399867296219 0.016154000535607338 0.016154000535607338 0.016194000840187073 0.01619499921798706 0.016234999522566795 0.016235001385211945 0.016276000067591667 0.016276000067591667 0.016315998509526253 0.01631700061261654 0.01631700061261654 0.016357000917196274 0.016398999840021133 0.016479000449180603 0.01647999882698059 0.01648000068962574 0.01660200022161007 0.01660200022161007 0.016642000526189804 0.016642000526189804 0.01684499904513359 0.0168869998306036 0.0168869998306036 0.016927000135183334 0.01700800098478794 0.01700800098478794 0.017090000212192535 0.01712999865412712 0.017170999199151993 0.017171001061797142 0.017211999744176865 0.01729300059378147 0.01782200112938881

Current
Mean: 0.017 ms
Stdev: 0.001 ms (3.6%)
Runs: 0.015625 0.015991000458598137 0.01603199914097786 0.016071999445557594 0.016112999990582466 0.016193998977541924 0.016235001385211945 0.016276000067591667 0.016276000067591667 0.016276000067591667 0.016316000372171402 0.01631700061261654 0.01631700061261654 0.01631700061261654 0.016356999054551125 0.016397999599575996 0.016397999599575996 0.01643799990415573 0.016439000144600868 0.016479000449180603 0.01647999882698059 0.016519999131560326 0.016520000994205475 0.016560999676585197 0.016600999981164932 0.016600999981164932 0.01660200022161007 0.01660200022161007 0.016642000526189804 0.016682999208569527 0.016682999208569527 0.016683001071214676 0.0167239997535944 0.0167239997535944 0.016764000058174133 0.016764000058174133 0.01676500029861927 0.016804998740553856 0.016805000603199005 0.016805000603199005 0.016885999590158463 0.016886001452803612 0.0168869998306036 0.016927000135183334 0.016927000135183334 0.016927000135183334 0.017007999122142792 0.017007999122142792 0.01700800098478794 0.017048999667167664 0.01712999865412712 0.017131000757217407 0.017131000757217407 0.017171001061797142 0.017211999744176865 0.017211999744176865 0.017253000289201736 0.01729300059378147 0.017334001138806343 0.017334001138806343 0.017334001138806343 0.017374999821186066 0.017374999821186066 0.0174150001257658 0.017456000670790672 0.017456000670790672 0.017496999353170395 0.017496999353170395 0.017497001215815544 0.01753699965775013 0.01753700152039528 0.01753700152039528 0.017578000202775 0.017618998885154724 0.017659001052379608 0.017741000279784203 0.01782199926674366 0.017862999811768532 0.01794399879872799 0.01794400066137314 0.017944999039173126 0.01810700073838234 0.018188001587986946 0.01822900027036667 0.018350999802350998 0.01839200034737587

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2023

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

@mountiny mountiny removed the DeployBlockerCash This issue or pull request should block deployment label Oct 6, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Oct 6, 2023

🚀 Deployed to production by https://github.com/Beamanator in version: 1.3.78-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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants