bugfix: core: Improve consistency while toggling from topic to stream. #948
Conversation
zulipbot
added
size: S
Abhirup-99
force-pushed
the
fix_cycle
branch
3 times, most recently
from
6bd2dd2 to
e3c20e3
Compare
|
@zulipbot add "PR needs review" |
Abhirup-99
changed the title
There was a problem hiding this comment.
Thanks for taking this up! The toggle behavior seems OK now 👍
As you rightly pointed out about the gap in 2 sets of indexed messages, I believe fixing that should fix this too?
I also found another fix locally for the stream toggle bug. Discussion link :)
Abhirup-99
force-pushed
the
fix_cycle
branch
2 times, most recently
from
8325475 to
3fd3975
Compare
The commit brings consistency while toggling from topic to streams. The original behaviour was to move to the last message in the stream, while toggling out. This changes it to zoom out to the context of the message selected in the stream. Tests added. Fixes zulip#934.
|
@Abhirup-99 I think we should avoid blowing away the message cache if we can - though I appreciate that this does look to be a solution - as we could end up fetching messages multiple times if nothing else. If we did take this approach, we would likely want to isolate the changes in the model in any case, as the change digs into the model from the controller; code like that still exists in ZT, but it's a lot easier to reason about otherwise. See the topic @Ezio-Sarthak linked for other discussion. |
neiljp
added
the
further discussion required
|
Heads up @Abhirup-99, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
Fixes #934.