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

New external storage disappears on screen rotation #1165

Closed
iadeelzafar opened this issue Apr 14, 2019 · 17 comments · Fixed by #1380
Closed

New external storage disappears on screen rotation #1165

iadeelzafar opened this issue Apr 14, 2019 · 17 comments · Fixed by #1380
Assignees
Labels
Milestone

Comments

@iadeelzafar
Copy link
Contributor

Describe the bug
Create a new external storage and it disappears on screen rotation.

Expected behavior
State should be saved.

Steps to reproduce the behavior:

  1. Go to settings>Storage>External.
  2. Click the '+' Button.
  3. Rotate the screen.

You will notice that newly created external storage disappears

Screenshots

Environment
Galaxy nexus API 24

@iadeelzafar
Copy link
Contributor Author

@mhutti1 Another closely related bug is that If user chooses this 0 bytes storage. Same bug as shown reappears when user tries to edit it(The 0 byte external storage is not shown in the alert dialog). To resolve this, I think we should not allow the user to select a 0 byte storage.

@soloturn
Copy link
Contributor

this is related to #1171 ?

@mhutti1
Copy link
Contributor

mhutti1 commented May 19, 2019

I don't think so.

@stale
Copy link

stale bot commented Jul 18, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot added the stale label Jul 18, 2019
@kelson42
Copy link
Collaborator

@macgills After you have make a few fixes on this dialog. Do you think by any chance, this bug might have been fixed?

@stale stale bot removed the stale label Jul 18, 2019
@macgills
Copy link
Contributor

No I wouldn't imagine it is. Just out of curiousity, why do we allow people to specify a new external storage? the use case is escaping me

@kelson42
Copy link
Collaborator

@mhutti1 is the best to answer this but AFAIK this was because @mhutti1 was not convinced that this tool was able to alwasy detect the storage properly. Personnaly, I would like to avoid that feature... which makes the whole stuff too complex.

@macgills
Copy link
Contributor

I would be much more in favour of getting rid of this feature and perfecting storage detection, very intense for a user to have to know their storage location

@kelson42
Copy link
Collaborator

@macgills I support this approach, if this can be done. @mhutti1 Your input here would be much appreciated as you have built all of this and has a longer experience of the problem.

@kelson42 kelson42 added the bug label Jul 18, 2019
@kelson42 kelson42 modified the milestone: 3.0 Jul 18, 2019
@macgills
Copy link
Contributor

By raising the minSDK to 19 (4.4, roughly 1500 users would not be able to upgrade) I am reasonably confident in a solution backed by Context.externalFilesDirs

@mhutti1
Copy link
Contributor

mhutti1 commented Jul 20, 2019

I don't think this specific issue is worth fixing imo. Its purely cosmetic. WRT the minSdk I would open another issue.

@mhutti1 mhutti1 closed this as completed Jul 20, 2019
@kelson42
Copy link
Collaborator

@mhutti1 Without new clearly open new issue, I would like to keep that ticket open? Could you please tell us more about this custom path input?

@kelson42 kelson42 reopened this Jul 21, 2019
@mhutti1
Copy link
Contributor

mhutti1 commented Jul 29, 2019

I don't think it saves state in general tbh. Its a bit of a hack.

@kelson42 kelson42 added this to the 3.0 milestone Jul 29, 2019
@kelson42
Copy link
Collaborator

@macgills I think you can carry in by removing the feature. Less convince by increasing the minimal SDKfor now, for that.

@mhutti1
Copy link
Contributor

mhutti1 commented Aug 15, 2019

@kelson42 I don't support removing this feature. It was added for a reason and this bug is really not very important, I wouldn't really class it is a bug at all to be honest. The feature is very bare bones for a reason, we don't want users to use it unless necessary. But as a last resort they can enter their own path and select it. Once it is selected it is used and displayed in the menu but won't appear in the selector because it was never meant to. If the user wants to use it after switching to another path they will need to enter it again.

@macgills
Copy link
Contributor

…. well I didn't see this comment until just now.
I have a PR removing it there. I don't think this feature has a big value add so I would be in favour of the removal

@kelson42
Copy link
Collaborator

@mhutti1 @macgills What is sure is that this feature should work fine and here we suffer at least from two weaknesses like reported by @iadeelzafar. I don't know how the problem should be fixed, but either we fix the current bug or we remove the flacky part of the this external storage selector... But this is assuming the path proposed by @BBT-SeanMacGillicuddy works to a really large extend. Hopefully it wil, I'm ready to take that risk for a release.

macgills added a commit that referenced this issue Aug 16, 2019
…tom-external

#1165 New external storage disappears on screen rotation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants