Skip to content
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

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

Open
leomoty opened this issue Jul 30, 2024 · 6 comments

Comments

@leomoty
Copy link
Contributor

leomoty commented Jul 30, 2024

          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 #670 (comment)

@leomoty leomoty changed the title Current folder middle button clickable but it has no text decoration Breadcrumb: Current folder middle button clickable but it has no text decoration Jul 30, 2024
@leomoty
Copy link
Contributor Author

leomoty commented Jul 31, 2024

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

@jelly
Copy link
Member

jelly commented Aug 1, 2024

@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
Copy link
Contributor Author

leomoty commented Aug 1, 2024

@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
Copy link
Member

garrett commented Aug 1, 2024

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
Copy link
Member

jelly commented Aug 1, 2024

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
Copy link
Member

garrett commented Aug 1, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants