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

Implement a Progress Bar widget. #2333

Merged
merged 23 commits into from
Apr 26, 2023
Merged

Implement a Progress Bar widget. #2333

merged 23 commits into from
Apr 26, 2023

Conversation

rodrigogiraoserrao
Copy link
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
Collaborator

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
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
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
Collaborator

@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.

src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
@rodrigogiraoserrao
Copy link
Contributor Author

Updated recording of the indeterminate effect with 15 FPS:

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

Copy link
Collaborator

@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.

docs/examples/widgets/progress_bar_isolated.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Outdated Show resolved Hide resolved
src/textual/widgets/_progress_bar.py Show resolved Hide resolved
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