Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic dimming functionality #2751

Closed
wants to merge 12 commits into from
Closed

Conversation

farwyler
Copy link
Contributor

@farwyler farwyler commented Jun 11, 2022

This adds a first iteration of color dimming to the editor.

You can:

  1. Dim the colors of all unfocused views
  2. Dim the colors in the background of Overlays (pickers, etc.)

The following dimming functions are implemented:

  • "dim" just set the modifier flag "dim" of all cells
  • "lighten" / "darken" actually change the shade of the fg/bg colors. this only works for themes applying RGB colors, not ANSI colors.

Config example:

[editor.dim] Section

Enable dimming in certain areas. Disabled by default. Enable by assigning:

  • 0 = set text modifier flag DIM
  • negative value = darken colors (-127 = 100% darker)
  • positive value = lighten colors (+127 = 100% lighter)

darken/lighten colors only works with themes that set rgb colors, not indexed colors.

Key Description Default
overlay-backdrops Enable dimming and set shade of content behind overlays. None
unfocused-views Enable dimming and set shade of unfocused editor views. None
[editor.dim]
# darken backdrop of overlays
overlay-backdrops = -25
# dim fg color of unfocused views
unfocused-views = 0

Screenshots:

Screenshot 2022-06-11 at 19 15 28
Screenshot 2022-06-11 at 19 16 09

Todo:

  • feedback
  • documentation
  • some optimizations

Looking for feedback.
Do we actually want this functionality in the tui?
Leave this to alternative frontends or plugins later?
Could this be used with #1187 ?

@sudormrfbin
Copy link
Member

I guess this is a nice to have, but a config for it seems overkill, especially the kind of dimming functions to use. If this does get merged, we would also want to avoid pulling in a dependency (palette) simply to dim colors.


It does feel like a good usability improvement after trying it out though, so I'm more or less on the fence now 😛

@CptPotato
Copy link
Contributor

I guess this is a nice to have, but a config for it seems overkill, especially the kind of dimming functions to use.

If I'm reading the palette source correctly, darken(x) is just lighten(-x) so the config could probably be simplified. Even if the palette crate isn't used the option could still be represented as a single value.

@farwyler
Copy link
Contributor Author

thanks @CptPotato and @sudormrfbin for the feedback.
i have simplified the implementation and removed the dependency.
new configuration looks like this.

@farwyler farwyler marked this pull request as ready for review June 14, 2022 18:42
helix-tui/src/buffer.rs Outdated Show resolved Hide resolved
helix-tui/src/buffer.rs Outdated Show resolved Hide resolved
helix-tui/src/buffer.rs Outdated Show resolved Hide resolved
@archseer
Copy link
Member

Doing dimming across an entire area after the fact seems a bit of a hack -- I'd rather calculate the dimming inside render_view, similar to how we treat active/unfocused views by passing a bool into that function.

@farwyler
Copy link
Contributor Author

farwyler commented Jul 2, 2022

@archseer You are right, the post processing is quite hacky. I wanted to keep the dimming out of the rendering functions if possible, as it introduces code complexity there for something that is a graphical gimmick, at best a slight UX improvement.

With bb4fec7 i've tried another an implementation that does the dimming ad-hoc, with just slight adjustments to the main render loop and a little state in the drawing buffer.

@kirawi kirawi added A-helix-term Area: Helix term improvements A-theme Area: Theme and appearence related S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2022
@kirawi kirawi added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 22, 2022
@pascalkuthe
Copy link
Member

clsoing this one as stale. Almost the entire rendering code has been removed since so this would need to start over from scratch. Thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements A-theme Area: Theme and appearence related S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants