swipe between convos

[aaravgarg]

swipe left and right on convo detail page to scroll back and forward

[gemini-code-assist]

Code Review

This pull request introduces a swipe gesture to navigate between conversations on the detail page, enhancing user experience with a smooth slide animation. The implementation correctly preserves the active tab across navigations. However, the review identified a critical bug in ConversationProvider related to potential null pointer exceptions and undesirable side effects in a getter, which needs to be addressed to prevent a potential crash and improve code quality. The changes otherwise represent a solid improvement to the application.

[gemini-code-assist]

This implementation contains a critical bug. The force unwrap operator (!) on groupedConversations[date]! will cause a null pointer exception and crash the application if the date key is not present in the groupedConversations map. The subsequent check for the key's existence occurs too late to prevent the crash. Furthermore, mutating the state by adding a conversation within a getter method is an undesirable side effect that makes the code harder to reason about.

[aaravgarg]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a well-implemented feature to swipe between conversations on the detail page. The use of Navigator.pushReplacement with a custom slide animation is appropriate, and edge cases like the action items tab have been thoughtfully handled. The refactoring in getConversationDateAndIndex is a good improvement that fixes a potential crash and removes a side effect. I've identified one high-severity performance issue in the new getAdjacentConversation method that could cause UI lag for users with a long conversation history, which also relates to ensuring cached data remains fresh. My detailed feedback is in the review comment.

[gemini-code-assist]

This line re-sorts all date keys from groupedConversations on every swipe. For users with a long history of conversations, this can become a performance bottleneck, causing UI jank during the swipe animation.

To optimize this, you should cache the sorted list of date keys and only re-calculate it when the underlying groupedConversations map changes.

Here's a suggested implementation approach:

1. Add a cached list to ConversationProvider:

   List<DateTime>? _sortedDateKeys;

2. Create a method to get or compute the sorted keys:

   List<DateTime> _getSortedDateKeys() {
     _sortedDateKeys ??= groupedConversations.keys.toList()..sort((a, b) => b.compareTo(a));
     return _sortedDateKeys!;
   }

3. Use this method in getAdjacentConversation:
   Replace the current line with:

   final sortedDates = _getSortedDateKeys();

4. Invalidate the cache:
   You must set _sortedDateKeys = null; in every method that modifies the groupedConversations map's keys. This includes methods like _groupConversationsByDateWithoutNotify, _groupSearchConvosByDateWithoutNotify, deleteConversationLocally, etc. This ensures the list is regenerated on the next access.

References

1. When using cached data, ensure it is re-validated or invalidated when the underlying source changes to prevent serving stale data.