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

combined feed: Tap behavior inconsistent in the channel portion of recipient headers #1368

Closed
chrisbobbe opened this issue Feb 20, 2025 · 3 comments
Labels
a-msglist The message-list screen, except what's label:a-content

Comments

@chrisbobbe
Copy link
Collaborator

When you tap anywhere in the channel part of a recipient header—

Image

it should take you to the channel feed. If your tap is in that area but not on the channel icon "#", the channel name, or the ">", then we actually take you to the page for the specific topic. That's confusing, because you meant to go to the channel.

This will also be relevant for the channel action sheet, which will be offered on long-press in this area:

Implementation

I believe the bug can be fixed by adding a line to the relevant GestureDetector:

diff --git lib/widgets/message_list.dart lib/widgets/message_list.dart
index 48c0d70b5..7d51d6fbd 100644
--- lib/widgets/message_list.dart
+++ lib/widgets/message_list.dart
@@ -1080,6 +1080,7 @@ class StreamMessageRecipientHeader extends StatelessWidget {
         ?? zulipLocalizations.unknownChannelName; // TODO(log)
 
       streamWidget = GestureDetector(
+        behavior: HitTestBehavior.opaque,
         onTap: () => Navigator.push(context,
           MessageListPage.buildRoute(context: context,
             narrow: ChannelNarrow(message.streamId))),

We also need a widget test for the fix; we won't merge a PR without this. The test should pass after the fix and fail before the fix.

@chrisbobbe chrisbobbe added the a-msglist The message-list screen, except what's label:a-content label Feb 20, 2025
@gnprice
Copy link
Member

gnprice commented Feb 21, 2025

Thanks! Is this the same as #1179?

If so, then let's copy over the added details from this report (including that diagram) and close this one to consolidate.

E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 21, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added a test that checks if tapping empty space in channel header
area correctly navigates to the channel feed.

Fixes zulip#1368.
@E-m-i-n-e-n-c-e
Copy link
Contributor

Hi I have worked on this. The issue was that gesture detector does not detect empty spaces properly if it's hit test behavior is not set to opaque. I have also added a test that taps 3 px above the chevron icon and checks if it navigates correctly to the channel feed. Currently there is 11 px empty space below and above the icon. The test should work correctly unless the padding is drastically reduced. We could also have it tap just 1 px above the icon instead and it should still theoretically work the same

E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 21, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added a test that checks if tapping empty space in channel header
area correctly navigates to the channel feed.

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 21, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added a test that checks if tapping empty space in channel header
area correctly navigates to the channel feed.

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 21, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added a test that checks if tapping empty space in channel header
area correctly navigates to the channel feed.

Fixes zulip#1368.
@chrisbobbe
Copy link
Collaborator Author

Thanks! Is this the same as #1179?

If so, then let's copy over the added details from this report (including that diagram) and close this one to consolidate.

Ah, yep! Done. @E-m-i-n-e-n-c-e, I see you've opened a PR pointing to this issue—please adjust it to point to #1179 instead; thanks.

E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 24, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added a test that checks if tapping empty space in channel header
area correctly navigates to the channel feed.

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 25, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added a test that checks if tapping empty space in channel header
area correctly navigates to the channel feed.

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 25, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added a test that checks if tapping empty space in channel header
area correctly navigates to the channel feed.

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 25, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added two tests to verify the behaviour

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 25, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added two tests to verify the behaviour

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 26, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Also added two tests to verify the behaviour

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 27, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 27, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Fixes zulip#1368.
E-m-i-n-e-n-c-e added a commit to E-m-i-n-e-n-c-e/zulip-flutter that referenced this issue Feb 28, 2025
Set gesture detecter behavior of streamWidget to HitTestBehavior.opaque
to handle taps in empty space around the header.

Fixes zulip#1368.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-msglist The message-list screen, except what's label:a-content
Projects
Status: Done
Development

No branches or pull requests

3 participants