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

Move the user is typing and other metadata back below the composer box #9276

Merged
merged 6 commits into from
Jun 2, 2022

Conversation

mountiny
Copy link
Contributor

@mountiny mountiny commented Jun 1, 2022

Thanks for the review!

Basically, this does not introduce much of a new code. I am just reverting these 2 PRs:

But I am keeping some things from the original refactoring as its own component for the OfflineIndicator as that is handy.

Details

We have decided to move the metadata back to above and below the composer box and make sure that the text is vertically aligned.

I am still using some of the refactoring introduced when the changes were made as they seemed appropriate and handy.

Fixed Issues

$ #9251

Tests / QA Steps

First set of test

  • Get some other account to type a message to you so you can see the typing indicator
  • Make sure it is back below the compose box and that it is correctly vertically aligned, looking something like this:

image

Second set of tests

  • Create some group chat
  • Make sure there is not large spacing between the last message and the composer box as it was here:
    image

Third set of tests

  • go offline

  • make sure the offline indicator appears below the composer box

  • Copy some large text and paste it in so that you get more than 60000 character

  • Confirm that the maximum character limit message shows up below the composer box too

  • Verify that no errors appear in the JS console

Screenshots

image

Group chat - notice the padding at the bottom of the chat to the composer box is back to normal:
image

Web

image

Mobile Web

simulator_screenshot_12C3C2E9-F3DF-4ABC-A044-08C11E002EE2

Desktop

iOS

Simulator Screen Shot - iPhone 13 - 2022-06-02 at 15 14 47

Android

@mountiny mountiny self-assigned this Jun 1, 2022
@melvin-bot
Copy link

melvin-bot bot commented Jun 2, 2022

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

1 similar comment
@melvin-bot
Copy link

melvin-bot bot commented Jun 2, 2022

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@mountiny mountiny marked this pull request as ready for review June 2, 2022 13:29
@mountiny mountiny requested a review from a team as a code owner June 2, 2022 13:29
@melvin-bot melvin-bot bot requested review from PauloGasparSv and removed request for a team June 2, 2022 13:29
@mountiny
Copy link
Contributor Author

mountiny commented Jun 2, 2022

cc @amyevans and @Luke9389 As you have reviewed/worked on the previous PRs if you would be interested in leaving a review on this one. Thank you!

@neil-marcellini neil-marcellini self-requested a review June 2, 2022 16:28
Copy link
Contributor

@neil-marcellini neil-marcellini left a comment

Choose a reason for hiding this comment

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

I have 2 questions, but they aren't blockers, and overall it looks great. Thanks!

src/components/ExceededCommentLength.js Show resolved Hide resolved
src/styles/styles.js Show resolved Hide resolved
@mountiny
Copy link
Contributor Author

mountiny commented Jun 2, 2022

@neil-marcellini Thanks for the review, I responded, let me know if you happy with it and we can merge.

@neil-marcellini
Copy link
Contributor

Yep, good to go! Merging.

@neil-marcellini neil-marcellini merged commit 9003b10 into main Jun 2, 2022
@neil-marcellini neil-marcellini deleted the vit-fixTheComposeBoxMetadata branch June 2, 2022 17:39
@mountiny
Copy link
Contributor Author

mountiny commented Jun 2, 2022

Thanks!

@OSBotify
Copy link
Contributor

OSBotify commented Jun 2, 2022

✋ 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 Jun 6, 2022

🚀 Deployed to staging by @neil-marcellini in version: 1.1.72-0 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Jun 8, 2022

🚀 Deployed to production by @yuwenmemon in version: 1.1.73-2 🚀

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants