feat(bar): Add support for custom icons

[NSPC911]

You can now super a textual.renderable.bar.Bar class and change HALF_BAR_LEFT, BAR, and HALF_BAR_RIGHT, before passing the class as a keyword argument in the textual.widgets.ProgressBar widget as bar_renderable. The keyword arg's name might be weird.

resolves #5350

Image:

Code Snippet

from textual.app import App, ComposeResult
from textual.renderables.bar import Bar as BarRenderable
from textual.widgets import ProgressBar
from textual.color import Gradient
class SignsRenderable(BarRenderable):
    HALF_BAR_LEFT = "-"
    BAR = "="
    HALF_BAR_RIGHT = "-"
class ArrowsRenderable(BarRenderable):
    HALF_BAR_LEFT = ">"
    BAR = "="
    HALF_BAR_RIGHT = ">"
class ThickBarRenderable(BarRenderable):
    HALF_BAR_LEFT = "▐"
    BAR = "█"
    HALF_BAR_RIGHT = "▌"
class SlashBarRenderable(BarRenderable):
    HALF_BAR_LEFT = "\\"
    BAR = "|"
    HALF_BAR_RIGHT = "/"
class Application(App):
    DEFAULT_CSS = """
    .bar--indeterminate { color: #bf616a !important; background: #2e3440 !important }
    """
    def compose(self) -> ComposeResult:
        gradient = Gradient.from_colors("#8fbcbb", "#88c0d0", "#81a1c1", "#5e81ac")
        for widget in [ThickBarRenderable, SignsRenderable, SlashBarRenderable, ArrowsRenderable]:
            bar = ProgressBar(gradient=gradient)
            bar.BAR_RENDERABLE = widget
            yield bar
        yield ProgressBar(gradient=gradient)
    def on_mount(self) -> None:
        self.progress_timer = self.set_interval(1 / 10, self.make_progress, pause=True)
        self.set_timer(4.0, self.start_progress)
    def start_progress(self) -> None:
        for bar in self.query(ProgressBar):
            bar.update(total=100)
        self.progress_timer.resume()
    def make_progress(self) -> None:
        for bar in self.query(ProgressBar):
            bar.advance(1)
Application().run()

Please review the following checklist.

• Docstrings on all new or modified functions / classes
• Updated documentation
• Updated CHANGELOG.md (where appropriate)

[willmcgugan]

Not convinced this is something that warrants being in the constructor. It feels more like an internal detail than configuration.

How about we expose the bar renderable as a classvar?

Something like this:

class FancyProgress(ProgressBar):
    BAR_RENDERABLE_CLASS = FancyBar

Then you wouldn't have to specify it everywhere you use the progress bar.

[NSPC911]

If the bar renderable is passed in as a class variable into the Bar element, what about the main ProgressBar element?

I'm not sure how the ProgressBar element can receive the Bar Renderable to be passed to the Bar widget as a class variable before being rendered

[NSPC911]

I'm also a tad bit confused on the comment, because your review comment is on the Bar element while your code example is for the ProgressBar element

[davidfokkema]

I like the idea of a classvar, but I see your problem, @NSPC911. It is ProgressBar -> Bar -> BarRenderable and you want to change the renderable. It feels a bit heavy-handed to create two classvars where every user should have to do:

class FancyBarRenderable(BarRenderable):
    ...

class FancyBar(Bar):
    BAR_RENDERABLE_CLASS = FancyBarRenderable
    
class FancyProgressBar(ProgressBar):
    BAR_CLASS = FancyBar

where the whole FancyBar is just needed to connect the dots between FancyProgressBar and FancyBarRenderable. Maybe use a classvar only on ProgressBar and do pass the BarRenderable into the constructor of Bar? So the user can do:

class FancyBarRenderable(BarRenderable):
    ...

class FancyProgressBar(ProgressBar):
    BAR_RENDERABLE_CLASS = FancyBarRenderable

[NSPC911]

How should ProgressBar send the renderable to the Bar widget without it having an input in the constructor of the Bar class?

[davidfokkema]

How should ProgressBar send the renderable to the Bar widget without it having an input in the constructor of the Bar class?

Don't use it in the init of ProgressBar, but do use it in the init of Bar. So you get the benefit of quickly defining your fancy progress bar without having to pass in the renderable every time you need it or needing to override the init. You just pass it as a class var. Internally, the ProgressBar does pass on the renderable from the classvar to the Bar widget.

[davidfokkema]

LGTM!

[davidfokkema]

I had two comments regarding type hinting, otherwise seems to work great!

[davidfokkema]

I see that you are force pushing commits to this PR instead of pushing new commits. I've never done that myself, but is there a benefit to that I'm unfamiliar with?

[NSPC911]

I see that you are force pushing commits to this PR instead of pushing new commits. I've never done that myself, but is there a benefit to that I'm unfamiliar with?

keeps changes to a single commit, I'm just used to doing this

[davidfokkema]

keeps changes to a single commit, I'm just used to doing this

The downside of force-pushing is that git complains when I want to test your new changes:

hint: You have divergent branches and need to specify how to reconcile them.
hint: You can do so by running one of the following commands sometime before
hint: your next pull:
hint:
hint:   git config pull.rebase false  # merge
hint:   git config pull.rebase true   # rebase
hint:   git config pull.ff only       # fast-forward only
hint:
hint: You can replace "git config" with "git config --global" to set a default
hint: preference for all repositories. You can also pass --rebase, --no-rebase,
hint: or --ff-only on the command line to override the configured default per
hint: invocation.
fatal: Need to specify how to reconcile divergent branches.

I'll force it and have another look.

[davidfokkema]

I think this is ready, pending @willmcgugan's blessing.

[NSPC911]

keeps changes to a single commit, I'm just used to doing this

The downside of force-pushing is that git complains when I want to test your new changes:

git fetch
git reset --hard <remote>/<branch>

usually solves it for me

[davidfokkema]

git fetch
git reset --hard <remote>/<branch>

usually solves it for me

Yeah, I can solve it. But I'm used to clicking on PR's, opening them in GitHub desktop, checking them out in Visual Studio Code, and that breaks hard. Also when you made additional changes I could not compare them. I had to go over the whole PR again because I couldn't keep track of recent changes. Diffing the commits would have me go into the git reflog. If you do push additional commits instead of force-pushing, people can check up on the most recent changes. GitHub itself (the PR UI and how these comments and code comments are structured and displayed) is built on that workflow as well. So while I think people should have their own workflows that suit them, when interacting with a large project I feel it is best to stick to the 'sanctioned' workflow.

[willmcgugan]

LGTM

[willmcgugan]

Thanks all. Will merge pending tests.

[NSPC911]

So weird, only 3.13 on Windows fails, and that's the python version I use...

[davidfokkema]

It's the flaky test, see #6032. That workaround has been merged earlier today.