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

[Feature] #1241 Do not automatically DOWNLOAD updates in metered networks #1252

Merged
merged 2 commits into from
Jan 13, 2024
Merged

[Feature] #1241 Do not automatically DOWNLOAD updates in metered networks #1252

merged 2 commits into from
Jan 13, 2024

Conversation

GopherTheCoder
Copy link
Contributor

Hello @NickeManarin ,
I've tried to implement this feature requested in #1241 , but I changed it a bit. Instead of don't check for updates, I only constrained the automatic download of new version.

The method to check if is in metered networks comes from here:
stackoverflow

But there brings a problem.
The WinRT API I used was introduced after Windows 10, so I had to change the target OS version to 10.0.17763.0, which will
likely make the product incompatible with old Windows version.
I'm still trying to find a way to make it version adaptive, any advice would be appreciated.
But for now I still want to create this PR for visibility.

Appreciate your time!

@GopherTheCoder
Copy link
Contributor Author

Seems pre-Windows 10 support will be dropped by upgrading to .NET 8.0
.NET 8 - Supported OS versions
So the compatibility issue seems gone. Merged to reflect the change.

@NickeManarin
Copy link
Owner

Hello @GopherTheCoder !
Thank you for the help :)

Do I need to setup something on my end?
Because it says that the namespace Windows.Networking.Connectivity doesn't exist.

@NickeManarin
Copy link
Owner

Oh okay...
I just needed to target the correct OS version.
I assumed that you done that.

image

@NickeManarin
Copy link
Owner

Do you mind resolving the conflicts and rebasing (dev)?

In UserSettings.cs & Settings.xaml: Added Boolean value "DownloadWithMeteredNetwork".
In Options.xaml & Options.xaml.cs: Added checkbox in Options binded with the value.
In StringResources: Added StringResources description and info, en & zh.
In Other.cs: Added method IsMeteredNetwork, using WinRT API.
In ScreenToGif.csproj: Since the API used in method was introduced in Windows 10 (10.0.10240.0), so I have to change projects target OS version.
In ApplicationViewModel.cs: Use the method with option value to determine if we download new version automatically.
@GopherTheCoder
Copy link
Contributor Author

Do you mind resolving the conflicts and rebasing (dev)?

Did the rebasing, not so familiar with Git, hope I did it right.

For the target OS version, I thought the minimum OS version will already be 10.0.17763.0 implictly, as target framework changed to .NET 8.0. Seems that's not the case, still need to explicitly specify that, I see you already did that in dev.

Sorry I didn't do a test last time, only merged in Github webUI. I gave it a try locally this time, everything looks good. Is there any proper testing for changes/PRs that I can do?

@NickeManarin NickeManarin merged commit b0ccee9 into NickeManarin:dev Jan 13, 2024
@NickeManarin
Copy link
Owner

Is there any proper testing for changes/PRs that I can do?

I'll do a test manually. Thank you. :)

@GopherTheCoder
Copy link
Contributor Author

Hooray to my first ever open source code contribution!

@GopherTheCoder GopherTheCoder deleted the feature/1241-metered-network branch January 15, 2024 02:03
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.

2 participants