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

fix: duplicated_indeterminate_progress_bar #8238

Merged

Conversation

gastoner
Copy link
Contributor

@gastoner gastoner commented Jul 26, 2024

What does this PR do?

Fixes duplicated indeterminate progress bar

Screenshot / video of UI

Before:

Screen.Recording.2024-07-23.141935.mp4

After:

Screen.Recording.2024-08-08.071555.mp4

What issues does this PR fix or reference?

Closes #8193

How to test this PR?

Settings -> Resources -> Select almost any extension -> Create new -> Create and scroll up (now there should be visible only one progress bar - the one connected with console)

  • Tests are covering the bug fix or the new feature

@gastoner gastoner requested review from benoitf and a team as code owners July 26, 2024 06:58
@gastoner gastoner requested review from cdrage and feloy and removed request for a team July 26, 2024 06:58
Copy link
Contributor

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

LGTM

@gastoner gastoner force-pushed the duplicated_indeterminate_progress_bar branch 4 times, most recently from 4e17f80 to cec37ac Compare July 26, 2024 08:46
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

it looks like inProgress= is used at various place like CreateVolume

and FormPage is exposing the property so I think there will be a problem.

@gastoner gastoner force-pushed the duplicated_indeterminate_progress_bar branch from cec37ac to 9c49655 Compare July 26, 2024 09:16
@benoitf benoitf requested a review from deboer-tim July 26, 2024 15:39
@deboer-tim
Copy link
Contributor

it looks like inProgress= is used at various place like CreateVolume

and FormPage is exposing the property so I think there will be a problem.

Yes, this would remove the (only) progress bar from several other pages.

@gastoner gastoner force-pushed the duplicated_indeterminate_progress_bar branch 2 times, most recently from 1b77b2c to b1ded2d Compare August 8, 2024 05:12
@gastoner
Copy link
Contributor Author

gastoner commented Aug 8, 2024

Is there some place in PD where is only the terminal with progress going, without the topbar progressbar?
@deboer-tim @benoitf @axel7083

@gastoner gastoner self-assigned this Aug 8, 2024
@deboer-tim
Copy link
Contributor

Is there some place in PD where is only the terminal with progress going, without the topbar progressbar? @deboer-tim @benoitf @axel7083

It looks like the only other place where the component is used in onboarding, and it doesn't look like there is another scrollbar there. You'll need to walk through or get feedback from one of the others to see if it still needs a scrollbar there or there is a more generic place like the header that should have one.

@benoitf benoitf dismissed their stale review August 14, 2024 14:41

old review

@gastoner gastoner force-pushed the duplicated_indeterminate_progress_bar branch from b1ded2d to 362974d Compare August 22, 2024 08:59
Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@gastoner gastoner merged commit 2a26a86 into containers:main Aug 22, 2024
7 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.13.0 milestone Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🚥 In Review
Development

Successfully merging this pull request may close these issues.

Duplicated indeterminate progress bar
6 participants