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

Fix attachment menu animation order #3793

Merged
merged 4 commits into from
Aug 23, 2021
Merged

Fix attachment menu animation order #3793

merged 4 commits into from
Aug 23, 2021

Conversation

s3n-w6i
Copy link
Contributor

@s3n-w6i s3n-w6i commented Aug 4, 2021

Pull Request Checklist

  • Changes has been tested on an Android device or Android emulator with API 21

Tested on device with Android 11, shouldn't matter (assuming it worked before 😉)

What it does

This PR changes the order in which the items in the attachement menu appear. Currently, the ones on the right are faster than the ones on the left. Because the '+'-Button to trigger the menu is on the left, this doesn't make sense.

This behavior is more noticable if animations are disabled, because the menu doesn't have the circular mask animation. That's why the videos are recorded with animations disabled.

Current animation

MP4_20210805_110437.mp4

New animation

recording_20210818_214043_Trim.mp4

@s3n-w6i s3n-w6i marked this pull request as draft August 4, 2021 22:02
s3n-w6i added 3 commits August 5, 2021 20:16
Signed-off-by: Simon Wazynski <37022952+pr0gr8mm3r@users.noreply.github.com>
@s3n-w6i s3n-w6i marked this pull request as ready for review August 18, 2021 19:54
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, you are right, the animation are better like this.

Previously the plus button was on the right and has been recently moved to the left.

As a small remark users with RTL languages does not have the bug at the moment, but after this PR will be merged they will see the bug. But this problem is not introduced by your PR.

@bmarty bmarty merged commit ecbf873 into element-hq:develop Aug 23, 2021
@bmarty
Copy link
Member

bmarty commented Aug 23, 2021

My bad in RTL, the icons are also mirrored, so it will be fine.

@s3n-w6i s3n-w6i deleted the patch-1 branch August 23, 2021 09:31
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.

2 participants