Add style variations to the progress bar widget #3143

petertalaber · 2023-08-22T16:27:16Z

Fixes #2920.

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

willmcgugan · 2023-08-22T16:32:03Z

Hi @petertalaber Any chance of some screenshots of the variants?

petertalaber · 2023-08-22T16:48:14Z

Sure! It would look like these:

Rainbow:

Rainbow color ranges can be modified via component classes attribute: bar--bar.color and bar--bar.complete.

Thickness:

petertalaber · 2023-08-22T19:39:19Z

Adding @rodrigogiraoserrao

rodrigogiraoserrao · 2023-08-22T19:55:19Z

Just to be clear, does the rainbow variant change colours as the progress increases, or are the colours changing regardless of the progress advancing?

petertalaber · 2023-08-23T07:11:06Z

It will change colors as the progress increases. In the example above it will start from blue via cyan, green, orange up to red according to the color circle in counterclockwise direction. The starting and ending color can be configured and this range is divided proportional to the percentage to show the proper color.

rodrigogiraoserrao

Thanks for your work!

I've left some questions and remarks, mostly about style – so that this code fits with the rest of the code base.

Feel free to ping me or re-request a review when you are done going through my comments.

src/textual/color.py

docs/widgets/progress_bar.md

rodrigogiraoserrao · 2023-08-23T16:40:52Z

src/textual/widgets/_progress_bar.py

+            )
+        return colorscheme
+
+    def validate_thickness(self, thickness: int) -> int:


When you create the literal type for valid thicknesses and the set of valid options, they can be used here.

When I said "it can be used here", I also meant in the typing sense; something like

Suggested change

def validate_thickness(self, thickness: int) -> int:

def validate_thickness(self, thickness: int) -> BarThickness:

src/textual/widgets/_progress_bar.py

rodrigogiraoserrao · 2023-08-23T16:48:45Z

src/textual/widgets/_progress_bar.py

+        self._colorscheme = colorscheme
+        self._thickness = thickness


Docstrings, please.

Also, how do you feel of making these attributes reactives?

Isn't it enough for those to be reactive only in Bar class and be normal variables in ProgressBar?

If they're normal variables in the progress bar, then if someone does something like progress_bar.thickness = 2, that won't updated automatically the thickness of the progress bar because Textual has no way of knowing that it should also update the thickness of the bar renderable.

rodrigogiraoserrao

Great work on implementing the changes, thanks.

I have a couple of follow-up questions.

src/textual/widgets/_progress_bar.py

rodrigogiraoserrao · 2023-08-28T13:16:14Z

src/textual/widgets/_progress_bar.py

+            )
+        return colorscheme
+
+    def validate_thickness(self, thickness: int) -> int:


When I said "it can be used here", I also meant in the typing sense; something like

Suggested change

def validate_thickness(self, thickness: int) -> int:

def validate_thickness(self, thickness: int) -> BarThickness:

src/textual/widgets/_progress_bar.py

rodrigogiraoserrao · 2023-09-04T09:57:06Z

src/textual/widgets/_progress_bar.py

@@ -59,9 +79,15 @@ class Bar(Widget, can_focus=False):
    """The percentage of progress that has been completed."""
    _start_time: float | None
    """The time when the widget started tracking progress."""
+    color_scheme = reactive(_DEFAULT_BAR_COLOR_SCHEME)
+    """The color scheme of the widget."""


Suggested change

"""The color scheme of the widget."""

"""The color scheme of the rule."""

I hope you don't mind me sticking my nose in, but shouldn't this be "bar" rather than "rule"?

rodrigogiraoserrao · 2023-09-04T09:57:12Z

src/textual/widgets/_progress_bar.py

+    color_scheme = reactive(_DEFAULT_BAR_COLOR_SCHEME)
+    """The color scheme of the widget."""
+    thickness = reactive(_DEFAULT_BAR_THICKNESS)
+    """The thickness of the widget."""


Suggested change

"""The thickness of the widget."""

"""The thickness of the rule."""

Ditto #3143 (comment)?

rodrigogiraoserrao · 2023-09-04T09:58:20Z

src/textual/widgets/_progress_bar.py

+        if color_scheme is None:
+            color_scheme = _DEFAULT_BAR_COLOR_SCHEME


So, the idea is that setting to None falls back to the default value? (And same thing for the thickness?)
If so, I think you should mention that in the docstrings for the reactives.

Yes. It is easier to use I guess if we don't have to deal with default constants around the code. If not set Bar will turn None into the default value.

rodrigogiraoserrao · 2023-09-04T10:00:08Z

I'll see if I can get someone else from the team to review this.
We're pretty tight on time with major work on Textual on the web, the TextArea widget, and the command palette, though, so I hope you understand that this may take a bit of time.

petertalaber · 2023-09-05T06:44:08Z

Sure. That Textual on the web seems cool. Don't bother with this. You will check this patch when you have time again.

- Rainbow scheme will update bar color based on percentage starting from .bar--bar.color up to .bar--complete.color in counterclockwise direction. - Bar thickness can have 3 styles: thin, normal or thick.

davep · 2023-11-01T11:44:41Z

I'm giving this an initial review, and I'm a wee bit confused by the files being used for the docs, especially when it comes to the stylesheets. There seems to be some tcss files that contain things that aren't illustrated in the example screenshot/code, and there also seems to be at least one instance of a stylesheet missing:

There's no screenshot in this, just code, and no stylesheet.

Would it be possible to get a once over of the state of the files in the documentation, and perhaps be sure that each of the files just contain what's necessary to show off what's being illustrated.

As an aside, there looks to be a couple of files named with a trailing underscore; is there a reason for that?

davep · 2023-11-01T11:49:33Z

docs/examples/widgets/progress_bar_styled_rainbow_.py

+    def key_9(self) -> None:
+        self._action_common_keypress(90)
+
+    def _action_common_keypress(self, progress: int) -> None:


It's a bit of a nit-pick, but given that this is intended for the docs, I think I'd be tempted to write this as slightly more idiomatic Textual code, using BINDINGS and calling on the action.

davep · 2023-11-01T11:51:44Z

docs/examples/widgets/progress_bar_styled_thickness_.py

+    def key_2(self) -> None:
+        self._action_common_keypress(2)
+
+    def _action_common_keypress(self, thickness: int) -> None:


As mentioned in another file: I think I'd be tempted to do this with BINDINGS and the action, it feels like it would be more "normal" for Textual code.

docs/widgets/progress_bar.md

davep · 2023-11-01T11:54:42Z

docs/widgets/progress_bar.md

 ## Styling the Progress Bar

 The progress bar is composed of three sub-widgets that can be styled independently:

 | Widget name        | ID            | Description                                                      |
 | ------------------ | ------------- | ---------------------------------------------------------------- |
-| `Bar`              | `#bar`        | The bar that visually represents the progress made.              |
+| `Bar`              | `#bar`        | Bar that visually represents the progress made.              |


Was there a specific reason for changing this? I feel it reads better before the change.

davep · 2023-11-01T11:56:56Z

docs/widgets/progress_bar.md

@@ -125,9 +188,9 @@ The progress bar is composed of three sub-widgets that can be styled independent

 | Name         | Type    | Default | Description                                                                                             |
 | ------------ | ------- | ------- | ------------------------------------------------------------------------------------------------------- |
-| `percentage` | `float  | None`   | The read-only percentage of progress that has been made. This is `None` if the `total` hasn't been set. |
+| `percentage` | `float` | `None`  | The read-only percentage of progress that has been made. This is `None` if the `total` hasn't been set. |


The type looks to be wrong here, if percentage can be None then the type should be float | None.

davep · 2023-11-01T11:59:13Z

docs/widgets/progress_bar.md

 | `progress`   | `float` | `0`     | The number of steps of progress already made.                                                           |
-| `total`      | `float  | None`   | The total number of steps that we are keeping track of.                                                 |
+| `total`      | `float` | `None`  | The total number of steps that we are keeping track of.                                                 |


The type looks wrong here, of total can be None then the type should be float | None.

davep · 2023-11-01T12:01:29Z

src/textual/color.py

+
+        Args:
+            destination: Another color.
+            factor: A blend factor, -1 -> 1.


Nit-pick: I'd spell out that it's "between -1 and 1", and perhaps make clear if it's exclusive or inclusive.

rodrigogiraoserrao · 2023-11-01T14:30:00Z

@davep the trailing underscores are there on purpose:
to make sure the docs screenshots of the progress bar are consistent, we need to “freeze” the progress bar.
So, a file progress_bar_example.py contains the code for a progress bar app with some styling, and that's the code that is shown to the user in the docs, but then we screenshot progress_bar_example_.py, which is the same code, plus a tiny cheat to make sure that the progress bar is frozen always in the same location.

Does this make sense?

Do you have a better idea of how this can be accomplished?

davep · 2023-11-01T14:48:17Z

Does this make sense?

Ahh hah! Okay, that makes more sense.

willmcgugan · 2024-07-10T14:15:52Z

I think this is probably too large of a change for too little benefit. Closing, with apologies to @petertalaber . Thanks for your hard work.

rodrigogiraoserrao suggested changes Aug 23, 2023

View reviewed changes

petertalaber force-pushed the main branch 2 times, most recently from 644ce92 to 10a8bde Compare August 27, 2023 15:45

rodrigogiraoserrao suggested changes Aug 28, 2023

View reviewed changes

petertalaber force-pushed the main branch from 10a8bde to 8d934f1 Compare September 3, 2023 19:23

rodrigogiraoserrao reviewed Sep 4, 2023

View reviewed changes

rodrigogiraoserrao requested review from rodrigogiraoserrao, davep and willmcgugan September 4, 2023 10:00

petertalaber force-pushed the main branch from 8d934f1 to bf1ad7e Compare September 5, 2023 06:37

petertalaber force-pushed the main branch from bf1ad7e to 8697dce Compare September 5, 2023 07:12

petertalaber force-pushed the main branch from 8697dce to bcc686b Compare October 9, 2023 17:44

orosam and others added 2 commits October 9, 2023 20:22

Add hsl_blend to Color to support hue transitions

f69ef53

Add new color scheme and thickness to progress bar

aaf0e08

- Rainbow scheme will update bar color based on percentage starting from .bar--bar.color up to .bar--complete.color in counterclockwise direction. - Bar thickness can have 3 styles: thin, normal or thick.

petertalaber force-pushed the main branch from bcc686b to aaf0e08 Compare October 9, 2023 19:39

davep reviewed Nov 1, 2023

View reviewed changes

willmcgugan closed this Jul 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add style variations to the progress bar widget #3143

Add style variations to the progress bar widget #3143

petertalaber commented Aug 22, 2023

willmcgugan commented Aug 22, 2023

petertalaber commented Aug 22, 2023

petertalaber commented Aug 22, 2023

rodrigogiraoserrao commented Aug 22, 2023

petertalaber commented Aug 23, 2023 •

edited

Loading

rodrigogiraoserrao left a comment

rodrigogiraoserrao Aug 23, 2023

petertalaber Aug 27, 2023

rodrigogiraoserrao Aug 28, 2023

rodrigogiraoserrao Aug 23, 2023

petertalaber Aug 27, 2023

rodrigogiraoserrao Aug 28, 2023

rodrigogiraoserrao left a comment

rodrigogiraoserrao Aug 28, 2023

rodrigogiraoserrao Sep 4, 2023

TomJGooding Sep 4, 2023

petertalaber Sep 5, 2023

rodrigogiraoserrao Sep 4, 2023

TomJGooding Sep 4, 2023

petertalaber Sep 5, 2023

rodrigogiraoserrao Sep 4, 2023

petertalaber Sep 5, 2023

rodrigogiraoserrao commented Sep 4, 2023

petertalaber commented Sep 5, 2023

davep commented Nov 1, 2023

davep Nov 1, 2023

davep Nov 1, 2023

davep Nov 1, 2023

davep Nov 1, 2023

davep Nov 1, 2023

davep Nov 1, 2023

rodrigogiraoserrao commented Nov 1, 2023

davep commented Nov 1, 2023

willmcgugan commented Jul 10, 2024

	def validate_thickness(self, thickness: int) -> int:
	def validate_thickness(self, thickness: int) -> BarThickness:

	"""The color scheme of the widget."""
	"""The color scheme of the rule."""

	"""The thickness of the widget."""
	"""The thickness of the rule."""

		if color_scheme is None:
		color_scheme = _DEFAULT_BAR_COLOR_SCHEME

Add style variations to the progress bar widget #3143

Add style variations to the progress bar widget #3143

Conversation

petertalaber commented Aug 22, 2023

willmcgugan commented Aug 22, 2023

petertalaber commented Aug 22, 2023

petertalaber commented Aug 22, 2023

rodrigogiraoserrao commented Aug 22, 2023

petertalaber commented Aug 23, 2023 • edited Loading

rodrigogiraoserrao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigogiraoserrao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigogiraoserrao commented Sep 4, 2023

petertalaber commented Sep 5, 2023

davep commented Nov 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigogiraoserrao commented Nov 1, 2023

davep commented Nov 1, 2023

willmcgugan commented Jul 10, 2024

petertalaber commented Aug 23, 2023 •

edited

Loading