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

Add support for (un)resolving topics in ZT.

Open srdeotarse opened this issue 2 years ago • 5 comments

What does this PR do? This PR adds support for (un)resolving topics in ZT via TopicInfoView popup menu when topic is highlighted in left_stream_bar and i key is pressed to toggle topic settings.

[WIP] #1075

Tested?

  • [ ] Manually
  • [ ] Existing tests (adapted, if necessary)
  • [ ] New tests added (for any new behavior)
  • [ ] Passed linting & tests (each commit)

Commit flow

First commit adds TOPIC_DESC shortcut to keys.py Second commit adds above shorcut to TopicButton in buttons.py Third commit still figuring out a way to update topic name with RESOLVED_TOPIC_PREFIX in model.py Fourth commit adds TopicInfoView to views.py Last commit adds show_topic_info function to core.py

Notes & Questions

How to update the topic name by adding or removing RESOLVED_TOPIC_PREFIX when Topic Resolved checkbox is toggled?

Interactions

Visual changes image

srdeotarse avatar Jun 20 '22 12:06 srdeotarse

I think it's better to remove Topic settings from the popup because when we add things like Mark all as read, it's not exactly a setting. Also maybe lets use a button, that says either Resolve Topic or Unresolve Topic, instead of a checkbox because when we add things like Edit Topic, it won't exactly be a checkbox.

Rohitth007 avatar Jun 30 '22 06:06 Rohitth007

@srdeotarse It'd also be good to get the hotkeys linting resolved somehow.

neiljp avatar Sep 03 '22 07:09 neiljp

The lack of blinking cursor looks good too :+1:

neiljp avatar Sep 04 '22 23:09 neiljp

@srdeotarse The last push was a good tidy following my review :+1: The first 3 commits were very close, so I just merged those with adjustments to commit messages, removed spaces at the start of some error strings, added a simple test case for no-edits in ZFL75 and the indexing point I mentioned at https://github.com/zulip/zulip-terminal/pull/1235#discussion_r962376941 (last commit of 3 was 3c75b96978a463edb6fb33707e66dfd692848ea1)

neiljp avatar Sep 10 '22 07:09 neiljp

Heads up @srdeotarse, we just merged some commits that conflict with the changes you 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 May 09 '23 03:05 zulipbot