Skip to content

views: Preserve focus and search on external pin events#1633

Open
maciej103 wants to merge 2 commits intozulip:mainfrom
maciej103:fix/1487-stream-pinning-focus-retention
Open

views: Preserve focus and search on external pin events#1633
maciej103 wants to merge 2 commits intozulip:mainfrom
maciej103:fix/1487-stream-pinning-focus-retention

Conversation

@maciej103
Copy link
Copy Markdown
Collaborator

What does this PR do, and why?

When a stream pin/unpin event arrives from an external client,
update_stream_view() rebuilt StreamsView from scratch. This reset
focus to the top of the list and cleared any active search. If a
pin event occurred while the search box was active, it also caused
an AttributeError due to search_lock not yet being initialized.

This fixes the bugs in #1487 by saving and restoring focus position
and search text around the rebuild, and by moving search_lock
initialization earlier in StreamsView.init().

External discussion & connections

How did you test this?

  • Adding automated tests for new behavior (or missing tests)

Self-review checklist for each commit

  • It is a minimal coherent idea
  • It has a commit summary following the documented style (title & body)
  • It has a commit summary describing the motivation and reasoning for the change
  • It individually passes linting and tests
  • It contains test additions for any new behavior

@maciej103 maciej103 force-pushed the fix/1487-stream-pinning-focus-retention branch 2 times, most recently from bcec1fd to 6e33d37 Compare April 10, 2026 03:49
Previously, search_lock was set after super().__init__(), which meant
an external pinning event could trigger the @Asynch update_streams
before search_lock existed, causing an AttributeError (see zulip#1487).
@maciej103 maciej103 force-pushed the fix/1487-stream-pinning-focus-retention branch from 6e33d37 to 4464764 Compare April 10, 2026 03:56
@zulipbot zulipbot added the size: L [Automatic label added by zulipbot] label Apr 10, 2026
@maciej103 maciej103 force-pushed the fix/1487-stream-pinning-focus-retention branch 2 times, most recently from 313d96e to 3ec673d Compare April 10, 2026 22:48
@zulipbot zulipbot added size: XL [Automatic label added by zulipbot] and removed size: L [Automatic label added by zulipbot] labels Apr 10, 2026
When a pin/unpin event arrived from an external client,
update_stream_view() rebuilt StreamsView from scratch, resetting
focus to the top and clearing any active search.

Save and restore the focus index and search text around the rebuild.
Clamp the focus index in case the list shrank after a pin event.

Fixes zulip#1487.
@maciej103 maciej103 force-pushed the fix/1487-stream-pinning-focus-retention branch from 3ec673d to 8c65828 Compare April 10, 2026 22:52
@zulipbot zulipbot added size: L [Automatic label added by zulipbot] and removed size: XL [Automatic label added by zulipbot] labels Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: L [Automatic label added by zulipbot]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI bugs from external pinning/unpinning stream events

2 participants