fix composer hidden while loading messages

[0xmiroslav]

Details

• introduce dummy composer by passing disabled prop in ReportActionCompose component
• introduce disabled prop in DragAndDrop component

Fixed Issues

$ #11856
PROPOSAL: #11856 (comment)

Tests

1. Login with any account
2. Open any chat
3. Switch between chats
4. Verify that composer view doesn't disappear
5. Refresh the page
6. Verify that composer view appears immediately

• Verify that no errors appear in the JS console

Offline tests

1. Login with any account
2. Disable network
3. Open any chat
4. Switch between chats
5. Verify that composer view doesn't disappear
6. Refresh the page
7. Verify that composer view appears immediately

QA Steps

1. Login with any account
2. Open any chat
3. Switch between chats
4. Verify that composer view doesn't disappear
5. Refresh the page
6. Verify that composer view appears immediately

• 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:
  • iOS / native
  • Android / native
  • iOS / Safari
  • Android / Chrome
  • MacOS / Chrome
  • MacOS / Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product was added in all src/languages/* files
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

web.mov

Mobile Web - Chrome

mchrome.mp4

Mobile Web - Safari

msafari.mp4

Desktop

desktop.mov

iOS

ios.mp4

Android

android.mp4

[melvin-bot]

@Beamanator @rushatgabhane One of you needs to 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]

[rushatgabhane]

hey, im ooo and also can't login
@Beamanator sorry that i can't review this PR

[0xmiroslav]

In summary: this PR = old merged PR + introducing disabled prop in DragAndDrop

[0xmiroslav]

bump for review @rushatgabhane @Beamanator

[Beamanator]

Just one comment, will start testing!

[Beamanator]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• I verified testing steps are clear and they cover the changes made in this PR
  • I verified the steps for local testing are in the Tests section
  • I verified the steps for Staging and/or Production testing are in the QA steps section
  • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
• I checked that screenshots or videos are included for tests on all platforms
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I verified that this PR follows the guidelines as stated in the Review Guidelines
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Screen.Recording.2022-12-21.at.4.57.00.PM.mov

Mobile Web - Chrome

Screen.Recording.2022-12-21.at.5.19.39.PM.mov

Mobile Web - Safari

Screen.Recording.2022-12-21.at.5.00.04.PM.mov

Desktop

Screen.Recording.2022-12-21.at.5.21.07.PM.mov

iOS

Screen.Recording.2022-12-21.at.5.08.10.PM.mov

Android

Screen.Recording.2022-12-21.at.5.18.35.PM.mov

[Beamanator]

Ok tested and pretty much everything looks good except:

1. On Web & Desktop, the dummy Report Action Compose has a Green outline, but not in any other platform
   • Also it looks like Report Action Compose doesn't auto-focus in iOS & Android - isn't it supposed to do that now in staging / production?
   • Asked if this is an issue here: https://expensify.slack.com/archives/C01GTK53T8Q/p1671637678612029
2. Should we take away the tooltip in the dummy composer? See my video for Desktop - I feel like we should, since it's not interactable when disabled
3. We're discussing this other topic: #13523 (comment)

[0xmiroslav]

1. On Web & Desktop, the dummy Report Action Compose has a Green outline
   I agree Green outline doesn't make sense when disabled

2. Should we take away the tooltip in the dummy composer? See my video for Desktop - I feel like we should, since it's not interactable when disabled

I don't think we need to care about this. When composer input is empty, send button is disabled. And this still shows tooltip in current version.

We may need to get suggestion from design team.
cc: @shawnborton

[shawnborton]

I think we should make the compose box look inactive (no green border for :focus) when we switch between chats, then it can become green when the chat is loaded and it's ready to take some input.

[0xmiroslav]

I think we should make the compose box look inactive (no green border for :focus) when we switch between chats, then it can become green when the chat is loaded and it's ready to take some input.

ok and what about tooltip when disabled?

[shawnborton]

Yeah, I agree that we probably don't need to worry about tooltips for this.

[0xmiroslav]

@Beamanator done all suggestions. please review

[thesahindia]

Here to help with the testing (#11856 (comment))
It's looking good, I am testing it now..

[thesahindia]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• I verified the correct issue is linked in the ### Fixed Issues section above
• I verified testing steps are clear and they cover the changes made in this PR
  • I verified the steps for local testing are in the Tests section
  • I verified the steps for Staging and/or Production testing are in the QA steps section
  • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
• I checked that screenshots or videos are included for tests on all platforms
• I included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• I verified proper code patterns were followed (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I verified that this PR follows the guidelines as stated in the Review Guidelines
• I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• If a new component is created I verified that:
  • A similar component doesn't exist in the codebase
  • All props are defined accurately and each prop has a /** comment above it */
  • The file is named correctly
  • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
  • The only data being stored in the state is data necessary for rendering and nothing else
  • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
  • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
  • All JSX used for rendering exists in the render method
  • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Screen.Recording.2022-12-28.at.1.58.31.AM.mov

Mobile Web - Chrome

Mobile Web - Safari

Desktop

Screen.Recording.2022-12-28.at.2.41.50.AM.mov

iOS

Screen.Recording.2022-12-28.at.3.10.13.AM.mov

Android

Screen.Recording.2022-12-28.at.3.06.14.AM.mov

Beam's OOO and his concerns were addressed

[OSBotify]

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

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
TTI	792.860 ms → 821.664 ms (+28.804 ms, +3.6%)
runJsBundle	178.531 ms → 182.226 ms (+3.695 ms, +2.1%)
regularAppStart	0.014 ms → 0.015 ms (+0.001 ms, +8.2%)
nativeLaunch	10.188 ms → 10.188 ms

Show details

Name	Duration
TTI	Baseline Mean: 792.860 ms Stdev: 24.898 ms (3.1%) Runs: 736.139406000264 753.618704999797 753.9584330003709 758.733543000184 761.0702569996938 766.1113900002092 769.0115000000224 775.7195859998465 782.5586660001427 784.9739279998466 786.4332619998604 787.1856960002333 787.6403719997033 788.0990730002522 790.7383179999888 790.8867180002853 794.7159709995613 799.6153920004144 801.2588679995388 801.4977249996737 802.797043999657 804.8192630000412 809.0230040000752 812.981882000342 816.1057719998062 818.2185709998012 821.6071370001882 826.9143040003255 830.0951220002025 831.6068690000102 834.5088499998674 Current Mean: 821.664 ms Stdev: 42.271 ms (5.1%) Runs: 749.1703549996018 761.5798990000039 771.5718000000343 772.2488249996677 772.3967850003392 779.427714000456 784.215594000183 789.8156209997833 797.5212899995968 800.1488370001316 802.0393369998783 805.5096869999543 805.8061899999157 808.7116360003129 810.0482529997826 819.5459120003507 820.583088000305 821.0178559999913 821.3502310002223 826.627105999738 829.0124369999394 834.9934050003067 836.7248759996146 844.0565750002861 858.9260609997436 867.4877399997786 882.854248999618 892.1391740003601 896.4255550000817 897.882837000303 911.7425330001861
runJsBundle	Baseline Mean: 178.531 ms Stdev: 15.833 ms (8.9%) Runs: 148 152 159 159 160 162 169 169 169 170 171 171 171 173 174 174 175 179 180 181 182 183 184 189 193 198 200 200 202 204 204 208 Current Mean: 182.226 ms Stdev: 22.422 ms (12.3%) Runs: 152 154 157 160 161 164 164 164 166 167 167 169 170 171 173 175 178 179 183 185 188 189 192 200 201 208 212 213 224 230 233
regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (7.5%) Runs: 0.01269499957561493 0.012817000038921833 0.013183999806642532 0.013224000111222267 0.0133050000295043 0.013306000269949436 0.0134680001065135 0.0134680001065135 0.013469000346958637 0.013549999333918095 0.013671999797224998 0.013793999329209328 0.013794000260531902 0.013875999487936497 0.013915999792516232 0.0139979999512434 0.014403999783098698 0.014404000714421272 0.01460800040513277 0.014688999392092228 0.0147299999371171 0.014851999469101429 0.014852000400424004 0.015178000554442406 0.01534000039100647 0.015666000545024872 0.015910000540316105 0.016032000072300434 0.016032000072300434 0.017211999744176865 Current Mean: 0.015 ms Stdev: 0.002 ms (12.8%) Runs: 0.013346000574529171 0.013590999878942966 0.013671999797224998 0.013793000020086765 0.013875000178813934 0.013915999792516232 0.013956000097095966 0.013956999406218529 0.014038999564945698 0.014078999869525433 0.014078999869525433 0.014119000174105167 0.014403999783098698 0.01444500032812357 0.01448499970138073 0.014526000246405602 0.01476999931037426 0.015054999850690365 0.015381000004708767 0.015381000004708767 0.015421999618411064 0.01574699953198433 0.0157880000770092 0.016030999831855297 0.017171000130474567 0.017211000435054302 0.0174150001257658 0.017741000279784203 0.017904000356793404 0.0183100001886487 0.020955000072717667 0.021037000231444836
nativeLaunch	Baseline Mean: 10.188 ms Stdev: 3.245 ms (31.8%) Runs: 7 7 7 7 7 8 8 8 8 8 8 8 8 8 9 9 9 9 10 10 11 11 11 11 13 13 13 13 14 14 19 20 Current Mean: 10.188 ms Stdev: 2.038 ms (20.0%) Runs: 7 7 8 8 8 8 8 9 9 9 9 9 9 9 9 10 10 10 11 11 11 11 11 12 12 12 12 12 12 13 15 15

[OSBotify]

🚀 Deployed to staging by @arosiclair in version: 1.2.46-0 🚀

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

[OSBotify]

🚀 Deployed to production by @roryabraham in version: 1.2.46-0 🚀

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