Skip to content

Feature: Display drive details when hovering over the sidebar #9734

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

Merged
merged 24 commits into from
Sep 16, 2022

Conversation

ferrariofilippo
Copy link
Contributor

Resolved

Details of Changes
-When inserting children in the DRIVE section, I check if these drives contains at least one file/folder
-When a file/folder is created/pasted, the DRIVE section is updated

Validation

  • Built and ran the app

@yaira2
Copy link
Member

yaira2 commented Aug 16, 2022

What happens if a user inserts a brand new usb drive? Is this specific to floppy discs and cds?

@ferrariofilippo
Copy link
Contributor Author

ferrariofilippo commented Aug 16, 2022

I tried with a formatted sd card and it was shown only in the HOME section, not in the sidebar.
When I added a file it was shown also in the sidebar

@yaira2
Copy link
Member

yaira2 commented Aug 16, 2022

I'm trying to understand the use case for this, why should they be hidden in the first place?

@ferrariofilippo
Copy link
Contributor Author

Sorry I misunderstood issue's request.
Now it's fixed for Floppy Discs and CDs only

@yaira2
Copy link
Member

yaira2 commented Aug 16, 2022

You did a great job but I'm still trying to understand the goal of the feature request.

@ferrariofilippo
Copy link
Contributor Author

Honestly, I think that this feature, as asked in the issue, is pretty outdated. Nowadays, few people still use floppy disks/CDs.
At first, I thought this feature would aim at all removable drives. In this case, I would have used it in order to keep my sidebar tidy when I have many SDs/USB sticks plugged in.
I'll explain myself better: I like to take photos and in my backpack, I have like 15 SDs. Of these, in a day I might fill 4 or 5. When I come back home I don't remember which I used, so I plug them into my PC in batches of 6.
It's not a game-changing feature, but it would save me some time and I'd use it

@yaira2
Copy link
Member

yaira2 commented Aug 16, 2022

That is a valid use case but the home page displays the drive space, I'm wondering if that's a better way to see which one has items in it.

@yaira2
Copy link
Member

yaira2 commented Aug 29, 2022

We used to display the item size when hovering over the sidebar items, perhaps we can bring that back.

@ferrariofilippo
Copy link
Contributor Author

ferrariofilippo commented Aug 30, 2022

Ok, I'll work on this. Should I add the feature #1271 ?
I have to revert changes, am I right?

@yaira2
Copy link
Member

yaira2 commented Aug 31, 2022

@ferrariofilippo that's correct, we need to also do this in a way that still displays the full item path.

@ferrariofilippo
Copy link
Contributor Author

I worked on this for a while. I found out that on the drive section there's no problem. Instead, calculating the size of "Favorites" folders takes too much time: calculating the size of 3 folders took me 5 minutes. I used the Files method, not StorageFolder.
Personally, I think we should add this feature only to drives.
Adding it to the recycle bin, might result in the same problem as with other folders: if you have too much trash it will take ages to calculate the size.
Let me know what you think I should do

@yaira2
Copy link
Member

yaira2 commented Sep 1, 2022

I'm happy to do this for just drives 👍

@yaira2
Copy link
Member

yaira2 commented Sep 6, 2022

@ferrariofilippo I'm happy with the current solution but another alternative is to provide a setting for this. Whichever solution you like more is fine with me.

@ferrariofilippo
Copy link
Contributor Author

I don't think adding a setting for this feature is worthy. Let me explain: showing the size of drives has little impact on performance, since the size is always calculated. Moreover, adding a new setting may result in having too many of them.
On the UX side, the tooltip still remains really small, so it should not be a problem for the user.
My porposal is to add the feature as is and wait for users' opinion: if some of them find it annoying and they want a setting for it I'll add it

@yaira2
Copy link
Member

yaira2 commented Sep 6, 2022

I don't think adding a setting for this feature is worthy. Let me explain: showing the size of drives has little impact on performance, since the size is always calculated. Moreover, adding a new setting may result in having too many of them. On the UX side, the tooltip still remains really small, so it should not be a problem for the user. My porposal is to add the feature as is and wait for users' opinion: if some of them find it annoying and they want a setting for it I'll add it

That's a fair decision, do you have any screenshots of how the tooltips look?

@ferrariofilippo
Copy link
Contributor Author

Here's the tooltip

Screenshot (34)

@yaira2
Copy link
Member

yaira2 commented Sep 7, 2022

Looks good! It might be a good idea to add Path: before the path so that it's consistent with the size option.

@yaira2 yaira2 changed the title Hide empty drive bays in the sidebar #9170 Display drive free space when hovering over sidebar items Sep 7, 2022
0x5bfa
0x5bfa previously approved these changes Sep 14, 2022
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

LGTM.

@yaira2
Copy link
Member

yaira2 commented Sep 15, 2022

  • Is there supposed to be a progress bar when hovering over OneDrive? It would be really great if we can do this!
  • Hovering over the network folder displays a blank tooltip.
  • "Path" is redundant now that we have the progress bar.

@ferrariofilippo
Copy link
Contributor Author

  • Is there supposed to be a progress bar when hovering over OneDrive? It would be really great if we can do this!

Like this?
drive
(With OneDrive it shows me my PC's free space, if you can confirm that I'll open an issue)

  • Hovering over the network folder displays a blank tooltip.

Can you send me a pic of what you see? (Here's what I see)
Network

  • "Path" is redundant now that we have the progress bar.

Do you mean reverting Path: C:\... to simply C:\... for folders in the FavouriteSection

@yaira2
Copy link
Member

yaira2 commented Sep 15, 2022

(With OneDrive it shows me my PC's free space, if you can confirm that I'll open an issue)

That should be fixed to show OneDrive storage or alternatively it can be hidden when it can't be fetched.

Can you send me a pic of what you see? (Here's what I see)

image

Do you mean reverting Path: C:... to simply C:... for folders in the FavouriteSection

Yes

@ferrariofilippo
Copy link
Contributor Author

That should be fixed to show OneDrive storage or alternatively it can be hidden when it can't be fetched.

As this issue involves even Properties window, I'll open an issue.


New tooltip desing (Drives and CloudDrives, should I add it for NetworkFolders too?):
Drive2

@yaira2
Copy link
Member

yaira2 commented Sep 15, 2022

New tooltip desing (Drives and CloudDrives, should I add it for NetworkFolders too?):

If it's possible to fetch the drive space, that would be really useful. Otherwise, progress bar should be hidden. (icon on the tooltip is redundant)

@ferrariofilippo
Copy link
Contributor Author

(icon on the tooltip is redundant)

Do you mean for NetworkFolders only, or for all sidebar's item?

@yaira2
Copy link
Member

yaira2 commented Sep 15, 2022

All sidebar's item 👍

@yaira2 yaira2 changed the title Display drive free space when hovering over sidebar items Feature: Display drive details when hovering over the sidebar Sep 16, 2022
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Sep 16, 2022
@yaira2 yaira2 merged commit fed8a76 into files-community:main Sep 16, 2022
@ferrariofilippo ferrariofilippo deleted the Files_Edit_Fix_#9170 branch September 21, 2022 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide empty drive bays in the sidebar
4 participants