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

3.25.0 introduced probably unintended UI changes #5965

Closed
2 tasks done
FeuRenard opened this issue Dec 17, 2016 · 21 comments
Closed
2 tasks done

3.25.0 introduced probably unintended UI changes #5965

FeuRenard opened this issue Dec 17, 2016 · 21 comments
Milestone

Comments

@FeuRenard
Copy link
Contributor

I have:


Bug description

6308e64 introduced this, let the screenshots speak for themselves.

Besides the changes you can see in the screenshots the animation when you send a message and the new bubble appears changed slightly, I think.

Screenshots

A wild Signal icon appears in...
... NewConversationActivity
device-2016-12-17-122927
... RecipientPreferenceActivity
device-2016-12-17-122942
... InviteActivity
device-2016-12-17-123004
... GiphyActivity
device-2016-12-17-123039

Mind the gap between back arrow and recipient, before 6308e64
bildschirmfoto von 2016-12-17 12-42-49
After 6308e64
bildschirmfoto von 2016-12-17 13-01-04

@2-4601 2-4601 added the bug label Dec 17, 2016
@Dyras
Copy link

Dyras commented Dec 17, 2016

I actually like the last change...

@moxie0 moxie0 added this to the 3.25.1 milestone Dec 17, 2016
@moxie0
Copy link
Contributor

moxie0 commented Dec 17, 2016

That's what we get for updating. Thanks!

@2-4601
Copy link
Contributor

2-4601 commented Dec 18, 2016

the animation when you send a message and the new bubble appears changed slightly, I think.

Yeah, noticed that too. It's weird but I guess one gets used to it quickly.

@moxie0 moxie0 closed this as completed in 4ff8de0 Dec 18, 2016
@moxie0 moxie0 reopened this Dec 18, 2016
@moxie0
Copy link
Contributor

moxie0 commented Dec 18, 2016

I got rid of the surprise app icon everywhere. The padding change is apparently to enforce the material standards. I think it's a waste of space on the contact search/selection activities so I disabled it there, but left it everywhere else.

That animation change is terrible! Anyone want to look into it?

@moxie0
Copy link
Contributor

moxie0 commented Dec 18, 2016

It looks like the ItemAnimator's "add" animation is getting canceled, so it displays immediately before the move animation for the items above it is complete. There are many changes to the data set in quick succession (pending, sent, delivered), and it seems that those changes are canceling the animation. Not sure why.

@moxie0 moxie0 closed this as completed in 71f8e9e Dec 19, 2016
@moxie0
Copy link
Contributor

moxie0 commented Dec 19, 2016

I've reverted the support library updates. I think we'll have to stop calling notifyDataSetChanged() in order to update them, which is kind of a big change, so I'm going to delay making it for now.

@nrizzio
Copy link
Contributor

nrizzio commented Jan 7, 2017

Is it just me, or are a lot of these problems back in 3.26.0? I think this issue should be re opened.

@FeuRenard
Copy link
Contributor Author

I spotted the wild Signal icon in GIF search and the additional space in several activities' action bars.

@moxie0 moxie0 reopened this Jan 7, 2017
@moxie0 moxie0 modified the milestones: 3.26.1, 3.25.1 Jan 7, 2017
@moxie0
Copy link
Contributor

moxie0 commented Jan 7, 2017

@nrizzio what are you seeing other than the signal icon in gif search?

moxie0 added a commit that referenced this issue Jan 7, 2017
@nrizzio
Copy link
Contributor

nrizzio commented Jan 7, 2017

@moxie0 the extra padding between the back arrow and recipient in a conversation thread is back, along with the awkward animation when a new message is sent/received.

@moxie0
Copy link
Contributor

moxie0 commented Jan 7, 2017

@nrizzio That extra padding is "intentional" now. You should be seeing the normal animation, though. What kind of conversation is it?

@nrizzio
Copy link
Contributor

nrizzio commented Jan 7, 2017

@moxie0 It's an encrypted Signal conversation. The problem exists in encrypted one-on-one and group chats. But the awkward animation will go away when using unencrypted text transport.
Here is a debug log: https://gist.github.com/6bdeb7312cefccec54619ee07746aab0

@FeuRenard
Copy link
Contributor Author

I see the old animation for Signal messages on Android 5.1.1.

@moxie0
Copy link
Contributor

moxie0 commented Jan 8, 2017

I decided to just remove the conversation item animations. There are some pretty bad recyclerview bugs that I can't work around, and it actually feels faster this way.

@moxie0 moxie0 closed this as completed Jan 8, 2017
@Trolldemorted
Copy link
Contributor

I have upgraded from 3.24.0 to the current master branch b7d4294 and i am pretty sure opening threads takes a little bit (something between 100 and 500ms i guess) longer.

I have held two signal instances on the same phone (same model, same OS version, same gapps) next to each other and opened the same thread multiple times: the 3.24.0 build has beaten the current master every time.

If i close a thread, press the home button and press the signal icon on the homescreen (must be homescreen) to open signal again, the next thread is opened significantly faster (i.e. long before the button-press animation finishes).

My local changes should not be related to this particular issue.

@moxie0
Copy link
Contributor

moxie0 commented Jan 8, 2017

@Trolldemorted if you can reproduce a performance issue please profile it and see where the time is going

@Trolldemorted
Copy link
Contributor

First i will have a look whether it is better after 5804213, since this sounds like it could be a recyclerview issue to me. If it is not, i will have a look!

@Trolldemorted
Copy link
Contributor

5804213 did not fix it. I played a little around with the android device monitor:

[top: "normal" thread selection", bottom: thread selection using method described above]
190ms

As you can see, during the "normal" thread selection we spend ~190ms more in android.view.ViewRootImpl.doTraversal, which would match my estimation of the delay. Any ideas what has caused this?

@moxie0
Copy link
Contributor

moxie0 commented Jan 10, 2017

@Trolldemorted sounds like a job for git bisect!

@Dyras
Copy link

Dyras commented Jan 11, 2017

I can confirm that on my Moto G4, opening a thread takes noticeably longer now.

@moxie0
Copy link
Contributor

moxie0 commented Jan 12, 2017

Bummer, if someone who can reproduce this wants to find the offending commit with git bisect, that'd probably help a bunch. Also let's open a new issue specifically for this performance degradation.

BLeQuerrec pushed a commit to SilenceIM/Silence that referenced this issue Feb 15, 2017
BLeQuerrec pushed a commit to SilenceIM/Silence that referenced this issue Feb 15, 2017
BLeQuerrec pushed a commit to SilenceIM/Silence that referenced this issue Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants