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

feat: make progress use lipgloss styles #543

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nervo
Copy link
Contributor

@nervo nervo commented Jun 24, 2024

Progress component is not cleanly usable in a lipgloss style driven project, because of its dependencies on termenv colors and color profile. Indeed, Both of them are not directly exposed by lipgloss styles..

Btw, i've managed to make it use lipgloss styles in a backward compatible way \o/

  • new FullStyle and EmtpyStyle variables are introduced, having the default foreground color of FullColor and EmptyColor
  • FullColor and EmptyColor are deprecated, make their default values to empty string. This way, if they are defined, we can set the foreground during rendering.
  • WithSolidFill now just apply foreground color to FullStyle
  • introduce WithFillStyles option which set both FullStyle and EmtpyStyle. It was inspired by WithFillCharacters, but can be easily split in both WithFullStyle and WithEmtpyStyle if you prefer
  • WithColorProfile is the most touchy part :) It recreates and make them inherit from themselves FullStyle and EmtpyStyle, but starting from the default renderer, and applies color profile to it

The rest is all about using FullStyle and EmtpyStyle to render runes, instead of termenv.String.

Happy comments :)

@nervo nervo requested a review from meowgorithm as a code owner June 24, 2024 06:41
@caarlos0 caarlos0 added the enhancement New feature or request label Aug 13, 2024
@meowgorithm
Copy link
Member

Hey again, @nervo! Nicely done here. We'll review this in a little more detail but generally speaking this looks awesome and is much needed, so thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants