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

Themes: Support shared background colors and re-enable transparency

Open neiljp opened this issue 2 years ago • 0 comments

What does this PR do, and why?

This follows PR #1410, which was split into a prep PR for this work after it became rather large.

Transparency was originally added in #378, but removed later and filed as #513.

Initial work towards re-enabling this feature was made in #555. However, the refactoring of theme handling to separate colors (vs color depth) and map them to styles, as well as add syntax highlighting, meant this work could not be re-used easily.

It is worth noting the differences between #555 and this PR:

  • #555 maintained a list of styles whose background would become transparent (when enabled)
  • this PR instead allows themes to use a dedicated color Enum when defining styles, which will be substituted with a transparent background (when enabled)

This approach has two benefits:

  • aside from transparency support, it enables themes to specify a common 'background', knowing that it is intended for that purpose and doesn't just 'happen' to have that same color; similarly, updating/exploring the background color is a simple adjustment
  • different parts of the UI can be set to remain opaque or not, in different themes.

Commit-wise, there are various refactoring and prep commits, with the main ones being:

  • Adding the common-background-color feature to zt_dark theme (refactor)
  • Enabling transparency in theme infrastructure
  • Connecting that toggle to new zuliprc settings and CLI options
  • Passing the option into the main application to allow viewing it in the About popup

Outstanding aspect(s)

  • [ ] Consider tests for new features (this PR)
  • [ ] Extend to other themes (followups)
  • [ ] Consider use of common-background feature to support multi-dark/light support (eg. gruvbox) (followups)
  • [ ] Document these changes (followups, including from #1410)

External discussion & connections

  • [ ] Discussed in #zulip-terminal in topic
  • [ ] Fully fixes #
  • [x] Partially fixes issue #513
  • [x] Builds upon previous unmerged work in PR #555
  • [x] Is a follow-up to work in PR #1410
  • [ ] Requires merge of PR #
  • [ ] Merge will enable work on #

How did you test this?

  • [x] Manually - Behavioral changes
  • [x] Manually - Visual changes
  • [x] Adapting existing automated tests
  • [ ] Adding automated tests for new behavior (or missing tests)
  • [ ] Existing automated tests should already cover this (only a refactor of tested code)

Self-review checklist for each commit

  • [x] It is a minimal coherent idea
  • [x] It has a commit summary following the documented style (title & body)
  • [x] It has a commit summary describing the motivation and reasoning for the change
  • [x] It individually passes linting and tests
  • [ ] It contains test additions for any new behavior
  • [x] It flows clearly from a previous branch commit, and/or prepares for the next commit

Visual changes

Initial attempts at a screenshot seem to ignore the effects, so please do give it a try yourself!

neiljp avatar Jul 06 '23 07:07 neiljp