Composer size when re-entering with unfinished multiline message #18089
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (39)
|
OmarBasem
approved these changes
Dec 7, 2023
churik
force-pushed
the
17361-cl-multiline-composer-reenter
branch
from
598d953 to
2b9fde1
Compare
|
@clauxx ISSUE 1: The composer jumps off the screen is reopened chat with a long messageSteps:
Actual result: FILE.2023-12-12.16.11.39.mp4OS: IOS, Android |
FFFra
approved these changes
Dec 12, 2023
|
Hey @churik, sorry for the long delay on this issue, was out sick last week. I fixed the issue you mentioned, but noticed that when re-entering the screen when there's an image added to the composer it focuses when opening the chat. This is different from the usual behavior, where it doesn't focus automatically. Is it intended? Also, noticed the composer bottom shadow was not visible for multiline messages when it's collapsed, so fixed that as well. |
77% of end-end tests have passedNot executed tests (1)Failed tests (5)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (6)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (37)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
|
churik
force-pushed
the
17361-cl-multiline-composer-reenter
branch
from
7bea8b9 to
7dbef3f
Compare
85% of end-end tests have passedFailed tests (3)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (4)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (41)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
|
@clauxx the behavior is definitely better, but still see that the composer jumps of and part of the text is hidden behind the keyboard until you start typing. The issue is reproducible on IOS only. FILE.2023-12-29.14.33.31.mp4 |
|
hey @clauxx any updates? still relevant? |
|
@flexsurfer just came back from vacation. Will look check it out tomorrow. |
| :style bottom-gradient-style}) | ||
| {:colors [(colors/theme-colors colors/white-opa-90 colors/neutral-95-opa-60) | ||
| (colors/theme-colors colors/white-opa-30 colors/neutral-95-opa-0) | ||
| ] |
There was a problem hiding this comment.
Hi @clauxx, Thank you very much for your PR.
- Please can you elaborate, how these changes are related to current issue?
- Also, Sorry I am having little hard time locating these values. Please can you provide an link?
There was a problem hiding this comment.
- It's an additional UI fix to the composer, where for multiline messages there should be a bottom shadow at the bottom when collapsed/minimized. I mentioned it to QA in a comment above, but will add it to the issue description as well.
- If you mean a design link, then the composer design should do it. But I didn't change the colors here, just adjusted the opacity and the gradient position so it looks according to the design.
There was a problem hiding this comment.
so it looks according to the design
Yes, that's what I am curious about. Please can you provide any reference what we are fixing here and what is not matching with design (maybe a screenshot or video). So we can know why it doesn't look design. Is it implementation issue or we are using different values then figma.
There was a problem hiding this comment.
Checked it against the designs and seems like this change is not necessary. I remember comparing it with the design at the time and the shadow being barely visible but it looks fine without these changes. Just the z-index fix should be enough to fix the bottom shadow, as before it wasn't visible at all. Thanks for being thorough with this 👍
|
@Parveshdhull since you're already working on refactoring the composer and I don't have many changes here, we could close this given it'll be fixed in the refactor. |
No, it's alright. Refactoring of composer is gradual and probably will not be complete (and address this issue) before release. Please feel free to continue working on this, currently, it doesn't create any conflicts. |
|
Hi @mariia-skrypnyk! I checked the issue that @churik mentioned, but can't reproduce it. Could you check if the issue is happening on your side? |
|
cc @pavloburykh |
|
@clauxx please rebase the branch and resolve the conflicts. After that we will continue testing. |
81% of end-end tests have passedFailed tests (6)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (3)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Passed tests (39)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
fix: composer space when expanding after re-entering fix: composer bottom shadow and maximized state atom fix: removing composer bottom gradient style changes
clauxx
force-pushed
the
17361-cl-multiline-composer-reenter
branch
from
8709cc3 to
c933bdd
Compare
Hi @clauxx ! Thanks for the update! Let me show you my steps:
I attached my and Pavlo's videos: video_2024-01-17_10-51-03.mp4video_2024-01-17_10-51-20.mp4Please, let me know if you need any additional help to catch this tricky issue! |
|
@mariia-skrypnyk Thanks for finding this! I think this issue is a bit different than the one reported by churik and the one I'm fixing here. Maybe would be best to have it as a separate issue, what do you think? |
No problem @clauxx. I add it as a new one:) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #17361
Summary
When re-entering the composer while there's an unfinished multiline message, the composer wouldn't show it properly. Even when focused, it only showed one line. Now, in the same scenario, it will show the first two lines of the message when not focused and the entire message when focused, as usual.
NOTE: additionally, added a fix to the composer bottom shadow, which should be shown when the composer is collapsed and has a multiline message.
Platforms
Areas that maybe impacted
Functional
status: ready