Skip to content

Locations refactory to use native Gio.AppIcon's (for GNOME 42 support) #1656

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 51 commits into from
May 15, 2022

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Jan 24, 2022

Refactor locations handling to have a more native Shell integration.

A part the various improvements and features, the main trigger for this was (as per the main commit message):

As per gjs 1.72 it won't be possible to inject an interface vfunc as it
used to work in previous versions, this would make gnome-shell to crash
as we may try to get the location "apps" ID which is now null (and not
customId anymore).

So, let's re-implement the things in a cleaner way by actually using
real custom AppInfo's that are still based on Gio.DesktopAppInfo
(beause the shell will still need to be such type), but that are
implementing the needed virtual functions of the parent interface.

As per this, we can finally use native shell mount operations to handle
mount options such as mounting encrypted disks.

@3v1n0 3v1n0 requested a review from ewlsh January 24, 2022 05:43
@3v1n0 3v1n0 force-pushed the locations-refactory branch from 0a39266 to ca44729 Compare January 24, 2022 05:44
@3v1n0 3v1n0 changed the title Locations refactory to use native Gio.AppIcon's Locations refactory to use native Gio.AppIcon's (for GNOME 42 support) Jan 24, 2022
@3v1n0 3v1n0 force-pushed the locations-refactory branch 2 times, most recently from e786dfc to 40d0858 Compare January 24, 2022 16:32
This was referenced Feb 4, 2022
@3v1n0 3v1n0 force-pushed the locations-refactory branch from 40d0858 to 5d26575 Compare February 23, 2022 16:57
@timocapa
Copy link

not sure if this is the right place for this, but some issues ive ran into so far:

dock doesn't hide properly and app names when hovered over don't disappear most of the time, making it actually kinda annoying to use

sorry if this is unrelated here (since im not sure what this commit does)

@stuarthayhurst
Copy link
Contributor

not sure if this is the right place for this, but some issues ive ran into so far:

dock doesn't hide properly and app names when hovered over don't disappear most of the time, making it actually kinda annoying to use

sorry if this is unrelated here (since im not sure what this commit does)

If you're referring to this branch specifically, I'm getting the same issues, using the standard release from EGO (Version 71), so I don't think it's related to this

@timocapa
Copy link

not sure if this is the right place for this, but some issues ive ran into so far:
dock doesn't hide properly and app names when hovered over don't disappear most of the time, making it actually kinda annoying to use
sorry if this is unrelated here (since im not sure what this commit does)

If you're referring to this branch specifically, I'm getting the same issues, using the standard release from EGO (Version 71), so I don't think it's related to this

I'm quite confident this was introduced in a Gnome change a while ago, thought I'd mention it anyway

@Etaash-mathamsetty
Copy link

could this get merged?

@schtobia
Copy link

I can confirm this fix for GNOME 42.0-2 as present in Debian sid.

@fancypantalons
Copy link

Confirmed here as well. Thank you @3v1n0 !

@rastersoft
Copy link

There is a bug in this patch: in the file docking.js, the property this._monitorIndex is replaced with this.monitorIndex, but there are several references kept to this._monitorIndex, specially in the Desktop Icons integration (which, BTW makes the integration to fail).

@phush0
Copy link

phush0 commented Apr 7, 2022

There is a bug in animation minimizing window. It will draw two animations one go to the left upper edge of screen and one minimize to dock. It is most visible with minimize Nautilus window

@martindmtrv
Copy link

Running this branch on my fresh Gnome 42 install all seems swell! Only issue I had was with the trash icon it had a white background. When disabling the "show trash icon" then re-enabling it seems to have fixed that issue

@OctoRocket
Copy link

Is this pull ready to go? Can't wait to use it on my computer!

@3v1n0 3v1n0 force-pushed the locations-refactory branch from 5d26575 to 1385d9e Compare April 11, 2022 13:26
@3v1n0 3v1n0 linked an issue Apr 11, 2022 that may be closed by this pull request
@3v1n0 3v1n0 linked an issue Apr 11, 2022 that may be closed by this pull request
3v1n0 and others added 16 commits April 28, 2022 17:36
Windows changes may not be processed at the same time we got a fm1Client
change now, so we should monitor apps signals instead of rely on what
happens on the backend
We don't care about exposing windows-changed signal if the file manager
app is wrapped to isolate devices. As we're handling signaling manually
anyways
Try to just read the trash item count file info before enumerating the
trash. It may be just cheaper and will work anyways
Since we've a native trash app info now we can handle the logic inside
that.

We need to handle data destruction from parent app wrapper though
We used to control volumes and volumes differently, while a mount can
always refer to its parent volume, so we can just handle both cases
using a more abstract class to handle all the mountable volumes so that
in case an unmounted volume gets mounted we can avoid creating a new
app but just update the state of the old one.

This will allow also to control more devices as the ones that are in a
not-mounted state
For some implementations we're getting the volume set after the mount so
let's ignore the volume in such case.

LP: #1960898
Some devices may not be visible in the launcher even if we can
technically list them, but it was not possible to mount them before
because we were using gio open only and we needed a mount location.

This is not the case anymore now, so we can just support showing all the
devices, whether they're mounted or not.
It's just faster to wait the last event not being cancelled in an idle
instead of doing them after the idle is ready.
If the user already requested to mount or umount a device, there's no
point to permit calling this again till the operation is not completed.

Notify about
So, for example still present the mount/unmount operations on devices
that are being mounted/unmounted but do not leave the user activate them
…-manager

When a new filemanager window is opened, we may assign it to nautilus
while we're still receiving its location infos, causing it to be
temporarily associated to the file manager app, instead to the actual
location app.

Causing some blinking, so let's use some quick timeout to wait the dbus
events to come before proceeding with the assignments.
An application can take some time to start (especially when performing
busy operations such as mounting a remote device), so in this case it's
better to mark the application as starting and only when we've windows
set as running.

This can be now easily implemented for the case
Sadly nautilus doesn't provide an API to open new windows when a
location is already opened, so using `gio open` can lead to focusing an
already opened location.

As per this, manually handle opening new windows if the command-line
supports --new-window parameter.
Because `g_file_monitor_set_rate_limit` doesn't work. It's a stub in
glib!

Fixes: https://launchpad.net/bugs/1962699
@3v1n0 3v1n0 force-pushed the locations-refactory branch from 6c032bc to ae11cb7 Compare April 28, 2022 15:38
We use the same upstream wording here, to avoid having to change it in
multiple places, so it has to be dynamic. Depending on the version we're
running on.

Fixes micheleg#1697
@3v1n0 3v1n0 force-pushed the locations-refactory branch from 095905a to d378382 Compare April 28, 2022 15:59
Apparently gjs was wrong in generating the camelCase name for variables
in Turkish (at least) as there 'i'.toLocaleUpperCase() is `İ`.

So let's workaround this, meanwhile gjs is properly fixed everywhere:
  https://gitlab.gnome.org/GNOME/gjs/-/merge_requests/742

Fixes micheleg#1687
@3v1n0 3v1n0 force-pushed the locations-refactory branch from f3d9d1a to a6447f2 Compare April 28, 2022 19:44
@3v1n0 3v1n0 linked an issue Apr 28, 2022 that may be closed by this pull request
@3v1n0
Copy link
Collaborator Author

3v1n0 commented Apr 29, 2022

@rastersoft I don't see your comment here, however the mentions of this._monitorIndex present are present only on functions/vfuncs overrides.

So they refer to the overridden type.

@rastersoft
Copy link

@3v1n0 Yes, I discovered it after posting the message, that's why I deleted it. Sorry for the noise.

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