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

"Unread messages" causes screen to jump when appearing/disappearing #4526

Open
WesleyAC opened this issue Mar 13, 2021 · 4 comments
Open

"Unread messages" causes screen to jump when appearing/disappearing #4526

WesleyAC opened this issue Mar 13, 2021 · 4 comments

Comments

@WesleyAC
Copy link
Contributor

The blue "Unread messages" bar at the top of the screen causes the message view window to noticeably jump when it appears/disappears. Instead, it should not affect the rendering of the message list at all, but smoothly appear on top of it, and then disappear again once the messages are read.

I think the fix for this will look like setting position: absolute on UnreadNotice, but I suspect it'll take a little bit of finagling to get it just right.

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Mar 13, 2021

Yeah; that jumping would be good to fix. In particular, I think the banner-exiting works like this (as far as I remember; I can double-check when I'm next at my regular dev setup):

  1. The banner begins its scale animation (which we're not fond of), decreasing in size while remaining centered in the rectangle that's laid out for it. The top of the WebView doesn't move; it remains aligned with where the bottom of the banner was before its exit animation.
  2. The banner progresses through its exiting animation. The top of the WebView still doesn't move.
  3. The banner finishes its exiting animation. The top of the WebView "jumps" up, so the WebView fills the space that was previously occupied by the banner.

(ISTR the banner's entrance works like that but in reverse; the top of the WebView "jumps" down to make room for the banner, then the banner animates in.)

I think the fix for this will look like setting position: absolute on UnreadNotice, but I suspect it'll take a little bit of finagling to get it just right.

One property of the current behavior that would be good to keep (Greg reminded me of how this works in #4209 (comment), which is why it comes to mind) is that the unread banner never obscures anything in the message list. That's currently done by stacking the unread banner and the WebView vertically in the layout, in the y-direction. I think if we used absolute positioning and a Z-index to overlay the banner on top of the WebView, we'd risk the banner obscuring a portion of the message list's content, making it impossible to see that portion until a user makes the banner go away by reading all messages.

I'd be very interested to know if we could fix the jumping, and keep the non-obscuring behavior, by ditching the current animation and (if we wanted there to be an animation at all) using React Native's LayoutAnimation API to have the banner's position and the WebView's height slide together, synchronized, while maintaining their stacking in the y-direction. I think a slide animation would be more appealing than the current scale animation.

I've recently had good results with that API in the lightbox, in #4442 (see in particular 3fa7a7f). You can basically tell it to "please schedule the next layout change (whatever that is) so that the elements move smoothly to their new positions." So we'd just to define positions for the elements (a) when the banner is visible, and (b) when it is not visible, and then just say that incantation just before the layout is told to change.

I think RN's Animated API is probably more fiddly than we need, and I wouldn't predict that it'd be more helpful here than LayoutAnimation.

@gnprice
Copy link
Member

gnprice commented Mar 15, 2021

I agree, this is a visible annoyance that'd be good to fix.

I think it may be tricky to work out a behavior here that's just right. Some constraints that would be good to satisfy are:

  • Things shouldn't jump around -- that's this issue.
  • As Chris says, the banner shouldn't obscure any of the message list so that you can't see it. In particular, if you scroll to the top of the history we want you to be able to see what's at the top (a date stamp, as it happens). Currently we successfully accomplish this.
  • When the banner appears and disappears, it shouldn't push the messages around. I think currently we accomplish this on net, in the end -- there's this jerking around, but the messages wind up at the same position after the jerking as they were before.

Another direction would be to try to replace the "N unread messages" banner with some other UI that doesn't pose these layout and animation challenges. I don't have concrete ideas at the moment for what that would be, but we could try to imagine one.

@WesleyAC
Copy link
Contributor Author

Some notes on CZO in #mobile-team > #M4526, unread bar jank.

As it turns out, this issue is kinda Big and Complicated, I'll split up some of my findings into smaller issues.

@Jumeb
Copy link
Contributor

Jumeb commented Nov 2, 2021

I agree, this is a visible annoyance that'd be good to fix.

I think it may be tricky to work out a behavior here that's just right. Some constraints that would be good to satisfy are:

  • Things shouldn't jump around -- that's this issue.

  • As Chris says, the banner shouldn't obscure any of the message list so that you can't see it. In particular, if you scroll to the top of the history we want you to be able to see what's at the top (a date stamp, as it happens). Currently we successfully accomplish this.

  • When the banner appears and disappears, it shouldn't push the messages around. I think currently we accomplish this on net, in the end -- there's this jerking around, but the messages wind up at the same position after the jerking as they were before.

Another direction would be to try to replace the "N unread messages" banner with some other UI that doesn't pose these layout and animation challenges. I don't have concrete ideas at the moment for what that would be, but we could try to imagine one.

I think the problem comes from switching the display property from flex to none. I discussed this with @alya here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants