Skip to content

Implement a Progress Bar widget.#2333

Merged
rodrigogiraoserrao merged 23 commits intomainfrom
progress-bar
Apr 26, 2023
Merged

Implement a Progress Bar widget.#2333
rodrigogiraoserrao merged 23 commits intomainfrom
progress-bar

Conversation

@rodrigogiraoserrao
Copy link
Copy Markdown
Contributor

@willmcgugan here is a first prototype that leans heavily on rich.progress.Progress.
Does this feel like the way forward?

Or should we implement things more from scratch so that we can do things like have component styles to style parts of the progress bar?

Screen.Recording.2023-04-19.at.18.24.50.mov

@willmcgugan
Copy link
Copy Markdown
Member

I think we should implement things from scratch, yes.

Functionality would be quite similar to Rich's Progress. Although simpler. It only needs to render a single progress, with percentage and optional ETA.

@rodrigogiraoserrao
Copy link
Copy Markdown
Contributor Author

rodrigogiraoserrao commented Apr 21, 2023

@willmcgugan I think things are going in a pretty good direction as of 8f2fae5.

But I have an issue I don't understand.
I don't know why I need this line:

self.percentage = None # Without this, assigning self.total breaks. 🤔

App to reproduce the issue
from textual.app import App, ComposeResult
from textual.widgets import ProgressBar, Footer


class MyApp(App[None]):
    BINDINGS = [
        ("a", "advance", "Advance by 1"),
        ("j", "jump", "Jump to 25"),
        ("u", "update", "Update to start"),
        ("d", "duplicate", "Duplicate total"),
    ]

    def compose(self) -> ComposeResult:
        self.pb = ProgressBar(total=None)
        yield self.pb
        yield Footer()

    def action_update(self):
        self.pb.update(total=100)

    def action_advance(self):
        self.pb.advance()

    def action_jump(self):
        self.pb.progress = 25

    def action_duplicate(self):
        self.pb.total *= 2


if __name__ == "__main__":
    MyApp().run()

Demo of the app with the progress bar:

Screen.Recording.2023-04-21.at.15.29.03.mov

@rodrigogiraoserrao rodrigogiraoserrao marked this pull request as ready for review April 24, 2023 16:42
@rodrigogiraoserrao
Copy link
Copy Markdown
Contributor Author

rodrigogiraoserrao commented Apr 24, 2023

Screenshot of a couple of progress bars (left and upper-left corner are indeterminate progress bars; not really on-brand for the app shown, but I tweaked the example just to get an indeterminate bar to show quickly.)

Untitled

The video demo above is current, up to the colour of the progress bar (which can be styled with CSS either way).

I still need to add tests, but I'll wait for @willmcgugan to determine what features he'd like to see/change before testing everything.

Copy link
Copy Markdown
Member

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality is about right I think. Looks good. But needs another pass at the implementation.

@rodrigogiraoserrao
Copy link
Copy Markdown
Contributor Author

Updated recording of the indeterminate effect with 15 FPS:

Screen.Recording.2023-04-25.at.18.13.42.mov

Copy link
Copy Markdown
Member

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! But that private attribute in the examples may be an issue.

We want examples to be show canonical usage, so we shouldn't be using private attributes.

I think we could fake it, show a screenshot with the hack to set the time, but link to the real example.

Copy link
Copy Markdown
Member

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just some nitpicks to consider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants