Show transfered file in folder as default action of notification (single click) #1039

HaMF · 2021-01-03T21:15:59Z

This adresses issue #1009, but probably needs some work. See issue discussion for details

I'm not fully happy with the behavior on success. The file manager should open with the file selected, but this is another issue. Behavior when the transfer is unsuccessful is yet untested

Using org.freedesktop.FileManager1.ShowItems the folder containing a file can be opened with the file selected. This can be used for the file transfer successful notification for example

…ication

sonnyp

Thanks! - and sorry for the late review

if we want to use this behavior when "Open Folder is clicked

Yes. This is better.

For the label, I think GNOME already has a wording for this action but I couldn't find it. Otherwise I'd suggest "Show in folder".

Given this is a GNOME Shell extension, I don't think we need the previous method as fallback.

if dbus is really the only way to achieve this (that's what I found, should work for nautilus and dolphin)

Yep, there are no wrapper exposed in GNOME libraries AFAIK

https://github.com/search?q=org.freedesktop.FileManager1&type=code

how tests can be added

I wouldn't say it's necessary here. But if you like checkout installed-tests/suites/components/. I guess you could listen on the bus and wait for what's of interested.

@andyholmes wdyt? any better idea for testing this?

how I can check the transfer-failed notification works

Perhaps send a large file and disconnect network midway. Why is it relevant?

sonnyp · 2021-06-19T06:26:53Z

src/service/device.js

@@ -733,6 +740,24 @@ var Device = GObject.registerClass({
        Gio.AppInfo.launch_default_for_uri_async(uri, null, null, null);
    }

+    openPathSelectFile(action, parameter) {
+        const path = parameter.unpack();
+        const uri = path.includes('://') ? path : `file://${path}`;


please use https://gjs-docs.gnome.org/gio20/gio.file instead

You can do something like

const file = Gio.File.new_for_path(path); file.get_uri()

@sonnyp Looking at this again, I wonder if that's really necessary. By the time it gets here, the uri has already been through Gio.File:

gnome-shell-extension-gsconnect/src/service/plugins/share.js

Lines 179 to 189 in 4208acc

buttons = [

{

label: _('Open Folder'),

action: 'openPath',

parameter: new GLib.Variant('s', file.get_parent().get_uri()),

},

{

label: _('Open File'),

action: 'openPath',

parameter: new GLib.Variant('s', file.get_uri()),

},

The URI coercion here is (as I read it) just a safety valve, in case someone adds a call to openPath() (or openPathSelectFile()) with a bare pathname. But in normal/expected usage, the parameter is the result of a file.get_uri() already. Round-tripping through Gio.File a second time may be overkill?

sonnyp · 2021-06-19T09:00:55Z

src/service/device.js

+            -1,
+            null,
+            null
+        );


looks like there should be an error to catch here

https://www.andyholmes.ca/articles/dbus-in-gjs.html#direct-calls

Logging the error is enough.

sonnyp · 2021-06-19T09:14:07Z

src/service/device.js

@@ -733,6 +740,24 @@ var Device = GObject.registerClass({
        Gio.AppInfo.launch_default_for_uri_async(uri, null, null, null);
    }

+    openPathSelectFile(action, parameter) {


consider renaming to showPathInFolder

andyholmes · 2021-06-19T20:12:18Z

I wouldn't say it's necessary here. But if you like checkout installed-tests/suites/components/. I guess you could listen on the bus and wait for what's of interested.

@andyholmes wdyt? any better idea for testing this?

I don't think there are many UI tests in the suite yet, those are kind of hard to do in a CI at the moment. Probably this one would just have to be tested locally.

ferdnyc · 2022-05-22T23:28:52Z

Jumping in way late on this, but I may pick it up if @HaMF doesn't plan to continue.

@HaMF

if we want to use this behavior when "Open Folder is clicked

@sonnyp

Yes. This is better.

For the label, I think GNOME already has a wording for this action but I couldn't find it. Otherwise I'd suggest "Show in folder".

The Nautilus wording (seen in both the Recent list and Search results) is "Open Item Location". Which I suppose may not technically be a folder, so I have to give them that. (It may be at the root of a removable drive, for example.)

However, when outside the file manager itself, the wording is usually different. Common are:

"Show in folder" (Chrome & Firefox download lists)
"Open Containing Folder" (Evince, GNOME Builder)
No text since there's just a 📁 icon with no tooltip, JFC. (GNOME Web)
No idea, it has never found a single photo on my system (which contains plenty of photos). (GNOME Photos)

HaMF · 2022-05-23T19:27:01Z

@ferdnyc Thanks for the research regarding the wording. Feel free to take it and finish it, I won't get around to it in the next days. If you haven't done so until next week when I'm back I'll look into it.

@sonnyp thanks for the review and the valuable pointers. Sorry must have missed the notification. I'll check in detail if ferdnyc does not pick it up

ferdnyc · 2022-10-19T09:08:05Z

I've submitted #1491 containing both @HaMF's commits from this PR (thanks, @HaMF!), as well as my further changes to address the review input here.

This can probably be closed in favor of #1491. I'll wait a bit, tho, so others have time to look over that PR and decide whether it's the right path.

HaMF added 4 commits January 3, 2021 22:03

Open containing folder on clicking file transfer successful notification

a981a7c

I'm not fully happy with the behavior on success. The file manager should open with the file selected, but this is another issue. Behavior when the transfer is unsuccessful is yet untested

Add action to open file browser at the files location using dbus

997fdab

Using org.freedesktop.FileManager1.ShowItems the folder containing a file can be opened with the file selected. This can be used for the file transfer successful notification for example

Use openPathSelectFile as default action in transfer successful notif…

ed51b56

…ication

Remove debug code

c591f47

HaMF mentioned this pull request Jan 3, 2021

Click on "Transfer successful" notification should open file or it's location with file selected #1009

Closed

HaMF changed the title ~~Implement showing transfered file in folder as default action of notification (single click)~~ Show transfered file in folder as default action of notification (single click) Jan 3, 2021

sonnyp self-requested a review April 9, 2021 23:14

HaMF marked this pull request as ready for review April 29, 2021 19:28

sonnyp requested changes Jun 19, 2021

View reviewed changes

sonnyp reviewed Jun 19, 2021

View reviewed changes

Merge branch 'main' into master

a1be3bb

andyholmes added the needs review A contribution that needs code review label Aug 5, 2022

ferdnyc mentioned this pull request Oct 19, 2022

Show transfered file in folder as default action of notification (replaces #1039) #1491

Merged

sonnyp closed this in #1491 Oct 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show transfered file in folder as default action of notification (single click) #1039

Show transfered file in folder as default action of notification (single click) #1039

HaMF commented Jan 3, 2021 •

edited by andyholmes

sonnyp left a comment •

edited

sonnyp Jun 19, 2021

ferdnyc Oct 17, 2022

sonnyp Jun 19, 2021

sonnyp Jun 19, 2021

andyholmes commented Jun 19, 2021

ferdnyc commented May 22, 2022 •

edited

HaMF commented May 23, 2022

ferdnyc commented Oct 19, 2022

	buttons = [
	{
	label: _('Open Folder'),
	action: 'openPath',
	parameter: new GLib.Variant('s', file.get_parent().get_uri()),
	},
	{
	label: _('Open File'),
	action: 'openPath',
	parameter: new GLib.Variant('s', file.get_uri()),
	},

Show transfered file in folder as default action of notification (single click) #1039

Show transfered file in folder as default action of notification (single click) #1039

Conversation

HaMF commented Jan 3, 2021 • edited by andyholmes

sonnyp left a comment • edited

Choose a reason for hiding this comment

sonnyp Jun 19, 2021

Choose a reason for hiding this comment

ferdnyc Oct 17, 2022

Choose a reason for hiding this comment

sonnyp Jun 19, 2021

Choose a reason for hiding this comment

sonnyp Jun 19, 2021

Choose a reason for hiding this comment

andyholmes commented Jun 19, 2021

ferdnyc commented May 22, 2022 • edited

HaMF commented May 23, 2022

ferdnyc commented Oct 19, 2022

HaMF commented Jan 3, 2021 •

edited by andyholmes

sonnyp left a comment •

edited

ferdnyc commented May 22, 2022 •

edited