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

Published 20 hours ago • verified publisher icon zulip

WIP: Feature: Add Spoiler support

Open Ezio-Sarthak opened this issue 3 years ago • 4 comments

@preetmishra Thanks for your primary work on this in a local branch, that helped me structuring this PR with separate commits and additional tests. This PR adds a basic support for spoilers in ZT.

Fixes #688.

Potential caveats

  • The links in spoilers are currently revealed in message info view, which isn't expected.

Commit flow:

  • boxes/themes: Add support for spoiler header markup in MessageBox.
  • boxes/views: Extract spoiler content in a variable in MessageBox.
  • core/views: Add SpoilerView class and corresponding show_* function.
  • boxes/core/views: Pass spoiler data to MsgInfoView popup helpers.
  • buttons: Add SpoilerButton class showing spoiler header.
  • views: Show SpoilerButton in message info view if present.

Screenshots/GIFs

spoiler_view

Ezio-Sarthak avatar Jul 02 '21 15:07 Ezio-Sarthak

@Ezio-Sarthak I just gave this a try - it looks good, and is a definite improvement over just showing the content like now! Thoughts:

  • It's quite easy to lose which spoiler I'm looking at, ie. basically the context. So, including at least the spoiler title, or maybe more details of the message, seems important?
  • I also felt confused when opening the spoiler popup and what to do "next" - I think most of our secondary popups have assigned keys, so exiting can return to the message info, for example.
  • The design suffers from the table limitations (I've not checked if you reuse the code), and long spoiler names break the table
  • Links in spoilers "leak out" via footlinks? (edit: you mentioned that in the description :+1:)

neiljp avatar Jul 04 '21 22:07 neiljp

Heads up @Ezio-Sarthak, 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

As per the issue, this set of features was discussed in the topic starting at https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/Spoilers!.20.23T688.20.23T1061.20.231173/near/904352.

neiljp avatar Feb 04 '24 00:02 neiljp

I'll be looking into this, will submit a WIP PR soon.

rsashank avatar Feb 04 '24 02:02 rsashank