Improve screen rotation handling in Open action menu #9135
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
devlearner
force-pushed
the
routeractivity-screen-rotate
branch
4 times, most recently
from
dbc9d65
to
a97c661
Compare
| // on the strength of "4. Fully transparent windows" without affecting the scrim of dialogs | ||
| final WindowManager.LayoutParams params = getWindow().getAttributes(); | ||
| params.alpha = 0f; | ||
| getWindow().setAttributes(params); |
There was a problem hiding this comment.
Thanks both for the heads up and reference! I've addressed it here, by simply setting LayoutParams.alpha to 0 to check off point 4 in their list. Do you think I'm getting it right? 🤔
From my testing (at least on my device) this won't have a side-effect on the scrim (which, in Material speak, refers to the dimmed background) behind AlertDialog/DialogFragments: the background will still be dimmed instead of fully transparent even with this put in, which is good for our purpose
|
(Oops, I messed up a bit while fast-forwarding the branch; the first five commits are the old commits from last time, sorry for the force-push noise.) Thanks both for the review and reference! I've updated it above 👍 So while we're at it, I've also taken the opportunity to revisit the last part of the issue, and tried to address it utilizing a retained fragment. It should now get everything covered from start to end for the duration and purpose of |
|
Kudos, SonarCloud Quality Gate passed! |
This seems to solve a bug where the Open action menu dialog does not appear the first time on cold start on older Android (8.0). This is also the order of things in MainActivity and probably good practice.
when orientation change is on foot
…ed from orientation change - Handle finish() call instead of passing around callbacks to setOnDismissListener() - Don't start over again if returning to DialogFragment before orientation change
and lengthened a bit to inform user to wait...
…ing windows so we won't hold up UI while fetching media info for Add to Playlist or Download actions lest user might think it freezes when in fact a network request is underway
to avoid adding it multiple times and ensure proper cleanup
pending result for openAddToPlaylistDialog() and openDownloadDialog() Despite marked deprecated, setRetainInstance(true) is probably our best bet (since a ViewModel is probably too overkill for our present purpose)
probably due to background restrictions on Android 10+
I thought it would have required an extra dependency; apparently that doesn't seem to be the case...
We provide visual feedback via a toast to the user that, well, they're supposed to wait; but with the benefit of the cache openAddToPlaylistDialog() may return (almost) immediately, which would render the toast otiose (if not a bit confusing). This commit improves that by cancelling the toast once the wait's over ... (by 'abusing' RxJava's ambWith(); ref on compose() and Transformer: https://blog.danlew.net/2015/03/02/dont-break-the-chain/ and for me, first time laying my hands at RxJava so kindly bear with me; open for suggestions)
Stypox
force-pushed
the
routeractivity-screen-rotate
branch
from
678ea51
to
dfcc464
Compare
Stypox
force-pushed
the
routeractivity-screen-rotate
branch
from
dfcc464
to
944e295
Compare
There was a problem hiding this comment.
Thank you! I tested and it seems to work well. The only inconsistency I noticed is that the initial selection menu stays shown even when the download dialog is showing above it, except after rotating, but this is not really a problem. I pushed a small commit that uses Optional for the activity context and simplifies a couple of other things. Thank you!
|
Kudos, SonarCloud Quality Gate passed! |
What is it?
Description of the changes in your PR
Currently when sharing to NewPipe (when set to
Always ask), the Open action menu dialog would rotate away on orientation change (sincefinish()would be called by onDismissListener during dialog cleanup thus preventingRouterActivityto be recreated when screen rotates). This PR tries to improve that by making the dialogs stay across screen rotations.Known issue: Ongoing network requests, however, won't survive config change (which would somehow require keeping track of theAddressed this using a retained fragment the other dayObservables across recreation of activity). If user selects Add to Playlist or Download and rotates away while stream info is being fetched, the network request underway would be dropped andRouterActivitywould start over again when recreated on orientation change. That would (unfortunately) be another PR for another day.Misc changes
To test:
Settings->Video & audio->Preferred 'open' actionis set toAlways ask(the default setting)Fixes the following issue(s)
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.
Due diligence