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 IProgress<int> and IProgress<string> in ProgressDialog #12

Closed
vpenades opened this issue Mar 25, 2020 · 5 comments · Fixed by #13
Closed

Implement IProgress<int> and IProgress<string> in ProgressDialog #12

vpenades opened this issue Mar 25, 2020 · 5 comments · Fixed by #13
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@vpenades
Copy link
Contributor

it could be very useful if ProgressDialog would implement these interfaces:

System.IProgress<int> to report progress percentage
System.IProgress<Text> to report textual progress

So the ProgressDialog object could be passed as a plain object to the running jobs, and casted to IProgress<int> and IProgress<Text> for progress reporting

@augustoproiete
Copy link
Member

Thanks @vpenades, that sounds like a great idea.

Let me know if you'd like to take a stab at it and send a PR, otherwise I'll get to it soon for the next release.

@augustoproiete augustoproiete added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Mar 25, 2020
@vpenades
Copy link
Contributor Author

PR ready...

@augustoproiete
Copy link
Member

@vpenades Ookii.Dialogs.Wpf now supports .NET 5 as well as .NET Core 3.1, so I thought I'd ping you in case you ever want to start discussing possible improvements to the IProgress<T> implementation

ps: Don't feel obligated to reply. This is just a friendly ping given your prior interest in this.

@vpenades
Copy link
Contributor Author

vpenades commented Nov 25, 2020

The use case most of the times is going to be background tasks initialized with an Object value that can be cast to multiple versions of IProgress<T> so they can report back the progress of the backgrond task.

I think the interface is not widely used because it's not very well known, but since IProgress<T> exists from quite some time, I did a small search to see how people is using it, and I found:

IProgress<float>  // progress reported in values between 0-1
IProgress<int> // progress reported in values between 0-100

It is very tempting to have a structure that keeps all the information required by the progress dialog (percent, text, description), but this must be avoided because that would mean the caller needs to fill that structure, which would force it to have a dependency on the ookii library, this is probably not desirable for background tasks, or pure code libraries that don't want to depend on an UI library.

So, in order to pass rich progress information, we need to use only BCL types and collections. Unfortunately, IProgress<T> does not define a standard.... but in general I guess everybody will expect (int Percent, string Text, string Description)

So, I would propose adding these interfaces:

IProgress<float> // by far, the implementation I've found most occurences searching on github.
IProgress< KeyValuePair<string,object> > // where key is the name of the property to set (percent, text, desc)

Additionally, the current IProgress<INT> and IProgress<STRING> could cache the content so they're not mutually exclusive; the current code already caches the progress, but it should do the same with the text. So a background task could call these interfaces consecutively without loosing the value set by the previous call on the other property.

@augustoproiete augustoproiete removed the good first issue Good for newcomers label Nov 25, 2020
@augustoproiete augustoproiete changed the title ProgressDialog should implement IProgress<T> Implement IProgress<int> and IProgress<string> in ProgressDialog Sep 6, 2021
@augustoproiete
Copy link
Member

Tracking the implementation of IProgress<T> on #58 and closing this one as completed in Ookii Dialogs v1.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Development

Successfully merging a pull request may close this issue.

2 participants