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

Dismiss download dialog correctly #6701

Merged
merged 2 commits into from
Jul 19, 2021

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Jul 19, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

The first commit just reorders some methods in the DownloadDialog class and adds some comment section separators.
The second commit fixes the DownloadDialog (which is a DialogFragment) dismissing according to this thread: every reference to getDialog().dismiss() is replaced with just dismiss(). I also removed references to getDialog().setOnDismissListener() and replaced them with a new onDismissListener in DownloadDialog, required by RouterActivity to be able to finish() itself when the dialog dismisses. Then I removed the part of code that used to finish() the RouterActivity inside the DownloadDialog (it was in the wrong place), as that's now done correctly by the listener.

Fixes the following issue(s)

Fixes #6667
Fixes #6256

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
@Pass1Vlax @khumarahn @skyGtm @TacoTheDank could you tell me if this works for you?

Due diligence

@Stypox Stypox changed the title Dismiss download dialog Dismiss download dialog correctly Jul 19, 2021
@ghost
Copy link

ghost commented Jul 19, 2021

I can't find the options in check, can you please send me a direct link?

@khumarahn
Copy link

@Pass1Vlax look for "artifacts".

I installed the new version. Was able to download a youtube video without problems.

@triallax triallax added bug Issue is related to a bug GUI Issue is related to the graphical user interface labels Jul 19, 2021
@skyGtm
Copy link

skyGtm commented Jul 19, 2021

Works good for both SAF enabled or disabled case.

@skyGtm
Copy link

skyGtm commented Jul 19, 2021

Might fix these issues:
#6687
#6665
#6256

@Stypox
Copy link
Member Author

Stypox commented Jul 19, 2021

@Pass1Vlax direct link

@ghost
Copy link

ghost commented Jul 19, 2021

@Pass1Vlax direct link

It works, Good job!

Copy link
Member

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

Looks good to me

@TobiGr TobiGr merged commit 9e2ece7 into TeamNewPipe:dev Jul 19, 2021
@TacoTheDank
Copy link
Member

Works! You're the best! :P

@khumarahn
Copy link

Thanks! Is it reasonable to ask for a bugfix release for this issue? It seems quite essential.

@ghost
Copy link

ghost commented Jul 19, 2021

Thanks! Is it reasonable to ask for a bugfix release for this issue? It seems quite essential.

I agree, it would help a lot of people that use this option

@pixel2user
Copy link

pixel2user commented Jul 20, 2021

Thanks! Is it reasonable to ask for a bugfix release for this issue? It seems quite essential.

@TeamNewPipe - Please release this fix as a hotfix-release on F-Droid. The bug makes NewPipe unusable on Android 10 and 11! :-(

https://github.com/TeamNewPipe/NewPipe/actions/runs/1049264498

@Stypox Stypox mentioned this pull request Jul 20, 2021
@Stypox Stypox deleted the dismiss-download-dialog branch August 4, 2022 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug GUI Issue is related to the graphical user interface
Projects
None yet
7 participants