Skip to content

Conversation

laevandus
Copy link
Contributor

@laevandus laevandus commented Sep 2, 2024

🔗 Issue Link

Resolves: PBE-5810

🎯 Goal

Fix the issue or marking a channel as read when the first unread message was one of the first not visible messages

🛠 Implementation

The fix is a workaround to the SwiftUI issue where the LazyVStack pushed to the NavigationStack has initially larger height and therefore trigger onAppear for more messages than it should. This causes ChatChannelViewModel to mark the channel as read because it thinks the user also saw the first unread message. Apple's feedback id is: FB15010770.

🧪 Testing

  1. Open a channel
  2. Scroll to the second message which was not visible on channel open
  3. Use mark unread using that message
  4. Go to channel list and open the same channel again

Before: Unread message pill appeared for a split second and the channel was marked as read
Expected: Unread message pill appears at the top of the channel view.

🎨 Changes

Note how in the before video the second opening on the channel marked it as read (the pill went away).

Before After
Before After

☑️ Checklist

  • I have signed the Stream CLA (required)
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Affected documentation updated (docusaurus, tutorial, CMS (task created)

@Stream-SDK-Bot
Copy link
Collaborator

Stream-SDK-Bot commented Sep 2, 2024

SDK Size

title develop branch diff status
StreamChatSwiftUI 7.06 MB 7.06 MB +2 KB 🟢

@laevandus laevandus enabled auto-merge (squash) September 4, 2024 06:11
Copy link

sonarqubecloud bot commented Sep 4, 2024

@laevandus laevandus merged commit 9163837 into develop Sep 4, 2024
11 checks passed
@laevandus laevandus deleted the fix/marking-read-when-first-unread-just-not-visible branch September 4, 2024 11:20
@Stream-SDK-Bot Stream-SDK-Bot mentioned this pull request Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants