zulip-terminal icon indicating copy to clipboard operation
zulip-terminal copied to clipboard

bugfix: Update stream-setting checkboxes from subscription events.

Open zee-bit opened this issue 3 years ago • 3 comments

This PR fixes the mismatch between stream settings if the StreamInfo popup is opened in ZT and subscription events are received from the server. The current approach registers a callback when the StreamInfoView is opened and looks for subscription events to trigger the callback, which in turn updates the checkbox UI.

Fixes #747.

zee-bit avatar Apr 05 '21 08:04 zee-bit

I'm wary from the description that a polling approach is the way to go with this, but rather that we do something like register a callback when opening the popup, which is triggered on events. That could be the starting point for a shift in how we glue parts of the application together, including loosening the coupling between the model and UI.

neiljp avatar Apr 05 '21 19:04 neiljp

I have changed my approach from polling to callback-based. This works well as a v1, but I need some feedback on improvement so that I can generalize this for other UI elements that depend on events for updation.

  • The first commit just adds an asynch function that will be responsible for updating the checkbox UI and will be passed as callback.
  • The second commit defines the logic for registering and processing callbacks from model that will eventually be used to update the UI.

zee-bit avatar Apr 19 '21 19:04 zee-bit

Heads up @zee-bit, 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 upstream/main branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar Aug 30 '21 23:08 zulipbot