Move chat dialogs to bottom sheet #9868
Merged
Conversation
This file contains hidden or 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
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (41)
|
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
2 times, most recently
from
d351b99 to
a380ba7
Compare
Ferossgp
changed the title
|
tested on iOS, everything's in order except the couple issues I pointed out. the bottom sheet animation works great now too! ✅ Fixed dragging #9846 issues: issues that need a follow up in separate PRs I made screen grabs from a video recording of how the first frame of animation looks like, the bottom sheet can start at wildly different positions of the animation, making it appear stuttering and laggy.
|
flexsurfer
approved these changes
Jan 22, 2020
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
from
682c5af to
d8d9b3a
Compare
errorists
approved these changes
Jan 22, 2020
60% of end-end tests have passedFailed tests (40)Click to expand
Passed tests (61)Click to expand |
62% of end-end tests have passedFailed tests (38)Click to expand
Passed tests (63)Click to expand |
|
Let's wait until eth.rod fleet will be fixed |
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
from
d8d9b3a to
8571e7a
Compare
96% of end-end tests have passedFailed tests (4)Click to expand
Passed tests (97)Click to expand |
|
@Serhy eth.prod fleet is fixed so this could be tested |
|
@Ferossgp Builds https://i.diawi.com/auRBcY and https://status-im-prs.ams3.digitaloceanspaces.com/StatusIm-200124-175215-8571e7-pr-universal.apk on iOS13.3 and Android 8.1 has been tested respectively. Issue 1'Fech 24h history' item has no icon asset on iOS (OK with Android)
Issue 2At the moment after any bottom collapsed, doesn't matter if it closed by tapping somewhere outside or closed by selecting one of it elements, - I have to tap on screen (like to bring back focus to app main screen) again and only after that further interaction with elements take effect. It happens with any bottom sheet. Please see Video and Reproduction below: Reproduction 1:
OR
Video: Issue 3:Delete chat does not work for public chat, neither from bottom-sheet after long-tap on public chat from Chats view nor from public chat's view called from Reproduction:
Video: |
|
@Serhy re: Issue 2. does the screen remain unresponsive permanently? during my testing I noticed the same behaviour but it only remained so for the first second or two after dismissing a bottom sheet. |
Yea, you are right, @errorists However then it should not be merged in RC until then :) Another issue I forgot to mention, (I just realised it's in RC as well but because in RC we don't have bottom-sheet as menu everywhere it could be reproduce with only one place):
|
|
Correct. @Ferossgp does your other work on bottom sheets fix Issue 2? |
|
@Serhy could you confirm that issue 2 is not present in current RC? |
No, it's fine with RC. Issue 2 ( |
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
from
bacc36c to
f2a060b
Compare
|
Thanks, @Serhy! All issues should be fixed in the latest commit. |
94% of end-end tests have passedFailed tests (6)Click to expand
Passed tests (92)Click to expand |
|
That looks good to me, @Ferossgp ! On iOS13 and Android8 and Android9 tested. Meanwhile, there is one weird issue I found. When from chat view Reproduction:
Video: https://drive.google.com/file/d/1cSBZW17IRB1WZpeLGZBBPZeQ3mipf-c-/view?usp=sharing (keep an eye on taps in video) |
|
One more issue (E2E actually failed with it): Reproduction:
Issue 5 |
|
@Ferossgp I've added some accessibility-labels to elements, hope it OK. |
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
from
2e1f1e0 to
4c89b84
Compare
Seems like there is a race condition. Should be fixed in this PR #9915 I can merge them together to avoid that available to users. |
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
2 times, most recently
from
b88a883 to
326a16d
Compare
This was referenced Feb 2, 2020
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
from
326a16d to
b4be65f
Compare
|
Last results for e2e: https://ethstatus.testrail.net/index.php?/runs/view/4926&group_by=cases:section_id&group_order=asc - failures are the same as in nightly. Tested:
Question to @errorists and to @Ferossgp : we still have pop-up menu when tapping links in the chat. Should it be addressed separately I believe? Also, testing of group chats is blocked by #10054 and #10057 |
|
@churik I've just looked at this modal and it is part of browser UI and it is reused in deep links for links which have |
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
from
b4be65f to
b1be292
Compare
Add bottom sheet to message long press Make whole bottom sheet panel dragable Fixes #9846 Use spring animation for bottom size of bottom sheet Remove extra border height from bottom sheet The height is already added to content height Remove extra set value for animation Timing and spring already mutates the animation value Reuse chat bottom sheet in chat list Update the size of new chat bottom sheet Add remove to group chat add chat id for clear history to be reused outside of current chat Fix public chat bottom sheet missing destructoring Replace icon for chat fetch history Could be rotated arrow up, but this requires special handling in icons or list item Fix remove public chat event Dismiss keyboard on sheet mount iOs rename arrow down icon Fix unusable screen after close of bottom shet The callback is called after 1.5 seconds after the animation starts. This happens because spring animation takes more time on animating post animation oscillations. Add accessibility labels Fix bad message destructoring add view profile on long message press Reset bottom value after animation Remove pending circle from avatar in chat sheets Do not show open profile on own messages Signed-off-by: Gheorghe Pinzaru <feross95@gmail.com>
Ferossgp
force-pushed
the
feature/move-chat-dialogs-to-bottom-sheet
branch
from
b1be292 to
40c1224
Compare
delete-merged-branch
bot
deleted the
feature/move-chat-dialogs-to-bottom-sheet
branch
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.
Also, add some UI/UX fixes to the bottom sheet component. Fixes #9701