cockpit-project / cockpit-files

A Featureful File Browser for Cockpit (Modernized and tested version of https://github.com/45Drives/cockpit-navigator)
GNU Lesser General Public License v2.1
52 stars 26 forks source link

Breadcrumb: Current folder middle button clickable but it has no text decoration #685

Open leomoty opened 2 months ago

leomoty commented 2 months ago
          the `pf-m-current` (aka current folder) is middle button clickable but it has no text decoration, that doesnt feel consistent

Originally posted by @leomoty in https://github.com/cockpit-project/cockpit-files/issues/670#issuecomment-2258245075

leomoty commented 2 months ago

@jelly is this intended? the breadcrumb is the only place where you can really reopen the same folder atm by middle clicking

jelly commented 2 months ago

@leomoty I can't say I copied the behavior we had before, note that cockpit isn't really made to open multiple windows as it will spawn another cockit-bridge. However I have no problems changing that behavior and showing an underline, @garrett what do you think?

leomoty commented 2 months ago

@leomoty I can't say I copied the behavior we had before

Alternatively we could just not let the user middle click the current portion of the breadcrumb?

garrett commented 2 months ago

Alternatively we could just not let the user middle click the current portion of the breadcrumb?

That landed in the recent breadcrumb changes.

And, sure, that doesn't have possible interaction issues with multi-select, so it makes sense as a good compromise.

However, can we make the href go to something like this URL:

https://localhost:9090/files#/?path=%252Fvar%252Fhome

Instead of:

https://localhost:9090/cockpit/@localhost/files/index.html#/?path=/var/home

So the page would be inside of Cockpit instead of outside of Cockpit? (The JavaScript location is fine; this is just having a different URL for the native browser clicks to open in a new tab to keep the shell.)

jelly commented 2 months ago

Alternatively we could just not let the user middle click the current portion of the breadcrumb?

That landed in the recent breadcrumb changes.

And, sure, that doesn't have possible interaction issues with multi-select, so it makes sense as a good compromise.

However, can we make the href go to something like this URL:

https://localhost:9090/files#/?path=%252Fvar%252Fhome

Instead of:

https://localhost:9090/cockpit/@localhost/files/index.html#/?path=/var/home

So the page would be inside of Cockpit instead of outside of Cockpit? (The JavaScript location is fine; this is just having a different URL for the native browser clicks to open in a new tab to keep the shell.)

We can't do that easily: https://github.com/cockpit-project/cockpit-files/blob/main/src/files-breadcrumbs.tsx#L222

garrett commented 2 months ago

So it can't be something like

`${window.location.origin}/files#/?path=${encodeURIComponent(url_path)}`;

Instead of

`${window.location.pathname}#/?path=${url_path}`;

for the HREF instead of the link? (You'd still use the second one for the location via JS; the first would only be for the href attribute for the <a> element.)

Is the problem the /files fragment? Or passing the href down the stack separately from the JS location?