Fix Progress component#145
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #145 +/- ##
==========================================
+ Coverage 92.53% 93.75% +1.21%
==========================================
Files 90 91 +1
Lines 1594 1665 +71
Branches 243 262 +19
==========================================
+ Hits 1475 1561 +86
+ Misses 99 88 -11
+ Partials 20 16 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
The implementation, especially the tests look overly complicated to me for what we want to achieve (display a progress indicator). But that may be due to my limited time.
I was not able to grasp what is done in pendingProgress.ts and what is actually tested in invokeCallbacks.test.ts.
And is the user supposed to set the visible flag? Why?
With that extra amount of code to be maintained it makes a lot sense to clearly document what has been developed here.
- Add a developer note (.md) about the current progress implementation
- Add docstrings to explain purpose and logic
- Comment tests
And
- Change name and meaning of property
visibleintohidden.
| size, | ||
| value, | ||
| variant, | ||
| visible = true, |
There was a problem hiding this comment.
visible is actually not a good choice as a component property. Many boolean component properties are optional and their default is therefore "falsy" (e.g. disabled). Hence this property should be called hidden, not visible.
| expect(screen.getByRole("progressbar")).not.toBeUndefined(); | ||
| }); | ||
|
|
||
| it("should not render when visible is false", () => { |
There was a problem hiding this comment.
| it("should not render when visible is false", () => { | |
| it("should not render when hidden", () => { |
See above.
| label?: string; | ||
| color?: string; | ||
| tooltip?: string; | ||
| visible?: boolean; |
| * Added `visible` support for progress components. Progress indicators can | ||
| now be hidden via `visible={false}` and are automatically shown while a | ||
| server-side callback with an output such as `Output("progress", "visible")` | ||
| is pending. |
| visible: bool | None = None | ||
| """If set, controls whether the component is visible.""" | ||
|
|
| * Added `visible` property to the base `Component` class, so components can | ||
| be shown or hidden through callback outputs such as | ||
| `Output("progress", "visible")`. |
2 participants
Description
This PR fixes the behaviour of progress components by adding
visiblesupport for the components and connecting it to callback execution state.Progress components can now be hidden with
visible=Falseand shown via callback outputs such as:When a server-side callback declares a progress component’s
visibleproperty as an output, chartlets now shows that progress indicator immediately while the callback request is pending. Once the callback completes, the normal callback output is applied, typically hiding the progress component again.Changes
visibleto the shared component state model.CircularProgressandLinearProgressrespectvisible={false}.invokeCallbacks().vega_test.py.Demo Panel
xcube-viewer panels demo
