GetStream / stream-chat-android

:speech_balloon: Android Chat SDK ➜ Stream Chat API. UI component libraries for chat apps. Kotlin & Jetpack Compose messaging SDK for Android chat
https://getstream.io/chat/sdk/android/
Other
1.45k stars 273 forks source link

Launching Message List in Dialog Fragment from Activity prevents scrolling #4215

Closed mruth1022 closed 2 years ago

mruth1022 commented 2 years ago

Describe the bug Due to the MessageListScrollHelper, scrolling is disabled when it is expected to be enabled, whenever there is a dialog fragment in the FragmentManager of the Activity of the RecyclerView.

There exists a style attribute as part of MessageListViewStyle called disableScrollWhenShowingDialog (which is enabled by default), which enforces a check in MessageListScrollHelper that will block scrolling whenever a DialogFragment is present in the recycler view's Activity's FragmentManager.

If the RecyclerView exists inside a DialogFragment that was opened from the Activity, then this check would trigger and disable scrolling unexpectedly.

We are able to work around the issue by overriding the default value of the disableScrollWhenShowingDialog from true to false, in the TransformStyle.messageListStyleTransformer.

SDK version

To Reproduce Steps to reproduce the behavior:

  1. Open a DialogFragment from an Activity.
  2. Use a MessageListView in that DialogFragment, or in any child Fragments in that DialogFragment.
  3. Attempt to scroll the list vertically.
  4. See that scrolling is disabled.

Expected behavior Scrolling should be enabled.

Device: Not significant

Screenshots Not applicable

filbabic commented 2 years ago

Hey @mruth1022!

We've actually had another similar issue being reported around this. By default, we disable scrolling when dialogs are present and visible, because in our default use cases, it makes sense to disable scrolling (overlay for selected messages, attachment pickers and similar).

The flag that you're using disableScrollWhenShowindDialog is the API that we exposed exclusively for your type of use case, when you're showing a custom dialog that should still allow scrolling.

So you're actually using the API correctly and I propose that we close this ticket, given that the workaround is helpful for you and there seem to be no other issues.

Edit: I misspoke about your use case, I was mentioning other similar use cases. In your specific one, you're hosing the RV inside a fragment, so the check fails (or blocks scroll). However, I don't think we should differentiate much between these two use cases as both seem quite similar and to some degree a bit more rare than what most of our users do.

The alternative would be to expose another flag/property you can set on the RV, which would let it know if it's being hosted within a DialogFragment, but IDK if that's going to be super visible or helpful to people. 🤔

Can you let me know if this works for you and if we can proceed with closing the issue?

Thanks! :]

mruth1022 commented 2 years ago

I think the main point I wanted to make is that it's not clear or obvious that the scrolling would be disabled in this case, and took a lot of confusion and frustrating debugging to eventually stumble upon that MessageListScrollHelper.

Maybe just improving the documentation or having the default style set disableScrollWhenShowingDialog to false, so that it's more opt-in only when consumers have dialog overlays, would be more clear.

filbabic commented 2 years ago

Hey @mruth1022!

I understand what you mean - while we cannot update the logic to enable scrolling (or set disable to false) by default, as it's a breaking change and it would break our default components which block scrolling while showing overlays, we can add extra documentation on the website about the scrolling behavior and how to control it!

I'll LYK once we update this so you can review!

Thanks!

filbabic commented 2 years ago

Hey @mruth1022 !

This guide has been merged and will be live with our next release: https://github.com/GetStream/stream-chat-android/pull/4233

Most likely to happen in the coming weeks as we're preparing a ton of improvements to the SDK! :]