RFC: Trash and Removable Device Icons #619
Conversation
|
I'd love to have this! It seems non-trivial ti implement though, and not having d-n-d is too bad (although nothing we can do about it). Quick comments:
Cheers! |
|
My handling of the icon is too simple. It's a themed icon and so has multiple names, and the name that actually yields an icon varies depending on theme, and indirectly that depends on the distro. I would need to iterate over the names and pick the first one that actually works. |
|
I've updated the pr with icon lookup; please try it out. |
|
Nice and quick fix! It's showing the correct icon now, at least for this drive I just tested. |
|
Another known problem is that opening a removable drive location creates a window that's mapped to Nautilus in the dock - so if you click on it again, you get a new window each time. Unity had this exact problem initially as well, so the Canonical folks patched in a dbus mechanism to get a mapping of XIDs to file locations which they used to recognise removable drive windows. The original bug report is here: https://bugs.launchpad.net/unity/+bug/887449 and there's a patch in the ubuntu sources for Nautilus. The next step on my side is to work out the best west to override the window -> app mappings that the shell is doing. |
|
Just pushed an incremental update to implement Unity's window mapping for the special icons. |
|
I've done some refactoring so that all the new logic is properly contained. I feel good about the code organisation now. |
|
Adding translation support. @micheleg: I consider this mergeable now, if you're interested in looking. |
|
Thanks a lot for the continuous work on this! I've been using it since last time with great success (and some minor quirks). I tested today the latest code from your branch, but I can't get the extension to load properly. Upon logging in, the shell freezes for ~10 seconds, and then loads with the extension disabled and this error in the log: This is on Gnome 3.26, Wayland. |
|
Thanks for the feedback. The special window matching logic that Ubuntu patches into nautilus is X11 specific as it identifies windows by XID. I have no idea what happens in Wayland and cannot easily test yet as my primary machine uses nvidia graphics. I'll be able to try it out in a couple of weeks when I can upgrade another machine to 17.10. I had hoped that it would just fail to report windows and wouldn't do something nasty like timeout. But one thing - if you log out and back in again, does it still block for 10 seconds? I had a problem where it would get stuck like this and I had to log out to fully restart gnome shell. |
|
Pushed a few improvements including what should be a fix for freezing if the DBus proxy decided to be slow to init. |
|
I tried to run the code on 3.22, but it is not compatible due to the use of the operator |
|
I've updated it to use push.apply() syntax. Should work now. |
|
Anyhow, I got the branch running. |
|
New update. I've fixed the js compatibility issues you mentioned, and added an improvement to deduplicate when a window is listed twice due to a location being open in multiple tabs. |
|
I had a chance to test in Wayland. As expected, window tracking doesn’t work, but it fails gracefully. It looks like the Nautilus patch would need enhancements. @micheleg, are you interested in merging this? Is there anything you’d like me to address? |
|
@franglais125: On drag and drop, we had "virtual placeholder" in unity if you are interested. Basically, the default of the gsettings key was: The running apps were the additional applications running but not in favorites, expo-icon, was the workspace expo mode (doesn't have any equivalent in GNOME Shell) and devices were those mounted and unmounted removable devices with an option to mount/unmount/unplug them. So, moving one item was moving the whole group. I don't say that's what you want to do here, just giving a possible solution we explored on Unity some years ago ;) |
|
Works okaish. I have a 2 issues:
I know that unity had some issues with decoupling nautilus with device icons but afaik they somehow figured it out in 16.04 or something like that. Also it seems to trigger hourglass cursor all the time (when emptying the trash or opening any attached drive). |
|
@0matgal0. I'm guessing you're using Wayland. As I said above, window matching doesn't work on Wayland. |
|
I've updated the pull request to adjust for the appIconIndicators refactoring. @micheleg can you let me know if you'd ever accept this change? or there's work you want me to do to it before you'd accept it? |
This change introduces a basic trash icon that can open and empty the trash. I did not attempt to implement any sort of drag and drop to put something in the trash. The implementation follows the path of least resistence and creates a DesktopAppInfo so that it can be represented by a regular AppIcon. That has particular implications - most significantly being that any actions offered by the icon have to be Execs of external programs, rather than code. In this case, we care about opening the trash and emptying it. We can use the 'gio' utility to open the trash using the default file manager, but emptying it is trickier. You can use gio to empty the trash but there is no confirmation dialog if you do this. Rather than implement such a dialog, I decided to use the Nautilus EmptyTrash dbus call; this triggers confirmation from Nautilus, but is obviously nautilus specific. Finally, I did not attempt to pin the icon to the bottom of the dash as Unity does. Given how elaborate the icon allocation logic is, I couldn't bring myself to tackle it.
Another Unity dock capability is showing icons for removable drives and devices. This change introduces such icons for these entities. As with the Trash, we back these with DesktopAppInfo, and implement the open/unmount/eject actions with the 'gio' utility. In Unity, icons are shown for both mounted and unmounted entities, and this behaviour is retained. Also retained is the practice of not exposing an unmount operation for ejectable entities. We also cannot show an unmounted icon if the entity has no activation root, but I believe that most entities of this type are ejectable so it's not a real problem. This limitation arises because the activation root is how we know where to mount the entity. Unlike the trash, the natural icon placement matches the behaviour in Unity.
In Unity, special logic is present that will map a Nautilus window the removable device or trash icon in the dock. The key to making this possible is a special dbus property that was patched into Nautilus that allows us to find the window where a location is open. Once we have access to this Nautilus information, we can then jump through a bunch of hoops to map the locations to MetaWindows and then a little special-casing logic, link our dock icons to those windows. Now, the special icons will have a running process highlight and window counts, and all the usual features of a running app. In a difference from Unity, I made no attempt to subtract the special location windows from the Nautilus app; that would be a bunch of work and the benefit is unclear. When run with an unpatched Nautilus, we will simply never see any linked windows, and the behaviour will be the same as without this change.
I realised that the location mapping mechanism in Nautilus maps to the exact uri and not the mount point, so if we want to match sub-directories, we need to do a prefix match rather than an exact string match on the location.
It appears that sometimes, initialisation of the DBus proxy can either take a long time or fail after a timeout. Let's avoid stalling gnome-shell by using the async form of initialisation. This isn't complex as we can attach signal handlers without waiting for init to complete.
A single window can show a location multiple times due to tabs. Let's deduplicate these so that the window count on the icon is accurate.
|
@philipl While I can't speak for @micheleg , on my side I ran out of time in the last few months and couldn't test your branch more thoroughly. I hope I'll get back to it some day, as it's pretty neat (and something I remember from Unity as well). If it's any indication, it took me ~6 months to get the multi-monitor branch merged. But it was nicely done eventually, with some nice rework by Michele. |
|
Just an idea: would it be possible to add some configurability?
Something like:
|
Make showing of these items optional.
philipl
force-pushed
the
master
branch
2 times, most recently
from
90c4ba2 to
f05d09e
Compare
|
Prefs Done! Thanks @franglais125. |
|
Gorgeous, thanks! For anyone reading, here is a screenshot of the prefs:
|
|
Hi, sorry for the late reply. Thanks for your work. I quickly tested your code a week ago and I was going to ask for some settings but I see that it has been done now. I still find it a bit incomplete ( without drag and drop support, yet the changes seems decently self-container so I'm keen on trying to merge this. Can you please summarize if there's any known corner case or issue? I didn't notice anything wrong on my side. I'll now try to look more thoroughly at your code. |
|
The issues I'm aware of:
Thanks for merging! |
|
It seems this was un-merged... Perhaps a local |
|
Yeah, there was a force push reported and now it's not on master. |
|
Yes, my fault, I pushed your branch by mistake while merging another patch. Sorry for the confusion. |
|
Can you reopen this pull request then? It won't let me do it. |
|
It seems I can't reopen it either... |
This set of changes adds Trash and Removable Device/Drive icons similar to what the Unity dock offers.
I don't consider it a complete change at this point; I think the code organisation isn't correct, if nothing else, but I'm particularly interested in feedback on the way I created the icons using DesktopAppInfo.
Other obvious shortcomings: