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: Display storage information for phones and tablets #16695

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ferrariofilippo
Copy link
Contributor

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Notes
I proposed two possible working solutions (one commented out):

  • the first one performs a double check
  • the second goes with only one check (which should be enough in most cases)

Tell me which one do you find more appropriate

Screenshots
Screenshot 2025-01-09 215554
Screenshot 2025-01-09 215559

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Open Files Home
  2. Enable Drives widget
  3. Connect a phone/tablet
  4. Check that storage is shown
  5. IMPORTANT: if you have the opportunity, test this feature using a phone with an SD card (may be source of bugs)

@yaira2 yaira2 changed the title Feature: Display smartphone/tablet storage size and free space Feature: Display storage information for phones and tablets Jan 9, 2025
Comment on lines +304 to +310
var propertiesSource = Root;
if (string.IsNullOrEmpty(Root.Path) &&
Path.StartsWith(@"\\?\", StringComparison.Ordinal) &&
(await Root.GetFoldersAsync())[0] is StorageFolder systemFolder)
{
propertiesSource = systemFolder;
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var propertiesSource = Root;
if (string.IsNullOrEmpty(Root.Path) &&
Path.StartsWith(@"\\?\", StringComparison.Ordinal) &&
(await Root.GetFoldersAsync())[0] is StorageFolder systemFolder)
{
propertiesSource = systemFolder;
}
var propertiesSource = (string.IsNullOrEmpty(Root.Path) &&
Path.StartsWith(@"\\?\", StringComparison.Ordinal) &&
(await Root.GetFoldersAsync())[0] is StorageFolder systemFolder)
? systemFolder
: Root;

@yaira2
Copy link
Member

yaira2 commented Jan 9, 2025

IMPORTANT: if you have the opportunity, test this feature using a phone with an SD card (may be source of bugs)

This appears to be an issue because the widget only displays storage information for the device, and not the SD card. Instead of showing this info on the drives widget, it might be better to display it within the folder view when you open the device in Files, just like File Explorer.

@ferrariofilippo
Copy link
Contributor Author

ferrariofilippo commented Jan 10, 2025

I'm not sure on how to proceed for implementation:

it might be better to display it within the folder view when you open the device in Files, just like File Explorer.

The issue is that it is a common folder, how should we display storage infos?
Should we add a new "secret" layout that is used only to display this kind of information?
Should we display the size only with tile view and/or add columns to the details layout?
An alternative could be to add the SD card to the Home page (basically we would fetch all the subfolders (phone and cards) and create a card for each one

@yaira2
Copy link
Member

yaira2 commented Jan 12, 2025

The details layout should be the easiest to update. We can add some additional columns and make them visible by default when opening the right type of drive. Regarding the implementation, is there a way to determine when to load the details?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Display smartphone/tablet storage size and free space
2 participants