Update animation for onboarding carousel. #16294
Merged
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
rahulpsingh
requested review from
Parveshdhull,
J-Son89,
flexsurfer,
cammellos and
ajayesivan
Jenkins BuildsClick to see older builds (44)
|
ajayesivan
reviewed
Jun 16, 2023
There was a problem hiding this comment.
Code LGTM!
@rahulpsingh I have a suggestion for improvement. Currently, when I release my hand in the middle of a swipe, the image instantly jumps into position. It would be great if we could add an animation for a smoother transition in such cases as well.
Also, I can swipe infinitely to the left, but not to the right.
Screen.Recording.2023-06-16.at.11.18.52.mov
|
@ajayesivan Sure, yes!! I'll ask @xAlisher for a recommendation about swiping animation and implement it.
This is an intended feature. |
J-Son89
reviewed
Jun 16, 2023
| [animate?])) | ||
|
|
||
| (defn cleanup-animation | ||
| [progress paused] | ||
| [progress paused?] |
0% of end-end tests have passedFailed tests (33)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
|
rahulpsingh
force-pushed
the
feature/16221-swiping-animation
branch
2 times, most recently
from
3d820a9 to
b9bd2d4
Compare
|
@rahulpsingh thanx for the PR. Please, take a look at the following issues: ISSUE 1 Delayed Slide Response in Carousel Swipe InteractionSteps:
Actual result: When interacting with the carousel by swiping the slides, there is a noticeable delay between the user's swipe gesture and the actual transition of the slide. This delay causes a poor response, as there is a significant lag between the user's movement and the slide switch. It appears that the slide transition is not happening immediately upon the user's swipe action, resulting in a delayed and unresponsive experience. telegram-cloud-document-2-5231220787289009455.1.mp4 |
ISSUE 2 Abrupt Slide Transition without Animation during Swipe InteractionSteps:
Actual result: During the swipe interaction of the slides, the transition between them is abrupt. This means that one slide abruptly changes to the next one without any visible transition or smooth animation between them. The moment of transition between slides is not perceptible, causing a jarring and sudden switch from one slide to another. This lack of smoothness in the slide animation negatively affects the overall user experience, as it hinders the ability to perceive and appreciate the visual flow and continuity of the content. telegram-cloud-document-2-5231220787289009490.mp4 |
rahulpsingh
force-pushed
the
feature/16221-swiping-animation
branch
3 times, most recently
from
0d0f328 to
26a31dd
Compare
0% of end-end tests have passedFailed tests (33)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
|
rahulpsingh
force-pushed
the
feature/16221-swiping-animation
branch
from
26a31dd to
3ca0c5f
Compare
rahulpsingh
force-pushed
the
feature/16221-swiping-animation
branch
from
3ca0c5f to
376facc
Compare
ajayesivan
approved these changes
Jun 24, 2023
rahulpsingh
force-pushed
the
feature/16221-swiping-animation
branch
4 times, most recently
from
4cf4b82 to
63a0f4f
Compare
|
@pavloburykh Both of the issues has been addressed. Please just have a look once!! |
|
@rahulpsingh thanx for the fixes! ISSUE 2 - fixed ISSUE 1 - IMO response is still bit delayed. But I would not block this PR by this issue. We will ask @status-im/design-team to try it out after merging and will log a followup if needed. Please also take a look at ISSUE 3. We can log it as a followup if this is not an easy fix, WDYT @rahulpsingh ? ISSUE 3 Long press does not stop the progress of the carouselActual result: in order to stop the progress of carousel user needs to at least slightly move the slide and then longpress. Pressing without moving does not result in stopping the progress. telegram-cloud-document-2-5249162029620538575.mp4 |
|
@rahulpsingh here is a blocker that needs to be fixed ISSUE 4 "No protocol IDeref. -deref defined for type null" crash on generating keysSteps:
Actual result:
telegram-cloud-document-2-5249162029620538585.mp4 |
|
@pavloburykh I'm looking at the crash issue(I think it's a side effect after rebase). Yes, I agree to the point that We can merge this PR and move to the design team and see their feedbacks as well. Thank you!! |
rahulpsingh
force-pushed
the
feature/16221-swiping-animation
branch
from
63a0f4f to
db5864b
Compare
|
@pavloburykh The 4th issue should as well be fixed. Please just have a look once!! |
|
@rahulpsingh thanx for the fix. Ready for merge. |
rahulpsingh
force-pushed
the
feature/16221-swiping-animation
branch
from
ae2728b to
ccfa66b
Compare
codemaster115
pushed a commit
that referenced
this pull request
Jul 7, 2023
* Update animation for onboarding carousel. * Fixed issues with long press while swipe. * Fixed issues after generating keys.
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 #16221
Summary
Recording
Testing notes
Platforms
Functional
Steps to test
status: ready