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

Support Mac specific keybinds #762

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tomasmatus
Copy link
Member

fixes: #757

@tomasmatus
Copy link
Member Author

tomasmatus commented Oct 3, 2024

I hope we can find a mac friend to test this 👀

Screen Shot 2024-10-03 at 17 10 16

const onKeyboardNav = (e: KeyboardEvent) => {
if (e.key === "L" && e.ctrlKey && !e.altKey) {
if (e.key === "L" && (isApple ? e.metaKey : e.ctrlKey) && !e.altKey) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test.

Comment on lines +20 to +21
const ctrlString = isApple ? "Command" : "Ctrl";
const altString = isApple ? "Option" : "Alt";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 added lines are not executed by any test.

};

const onKeyboardNav = (e: KeyboardEvent) => {
const ctrlKey = isApple ? e.metaKey : e.ctrlKey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage complains as it checks both sides of the branch, so isApple is never true so no 💯 code coverage.

@mac2net
Copy link

mac2net commented Oct 3, 2024

I am a Mac friend. I see you want the keyboard commands tested?
But on what exactly?
I currently only install Cockpit and addons though the repository on to Fedora.
I run Fedora 40, 41 beta and Rawhide in VMs on a Minisforum UM780 XTX (AMD 7840HS)
I guess in order to test this I need to install the development version of Files... and also Cockpit?
How do I do that under the circumstances?
I can test this over the weekend, if that's OK.
I use quite a bit of software related to system management and web development so there are a number of utilities fighting for keyboard shortcuts amongst themselves and with MacOS as well.
Please advise if you want me to proceed.
Cheers Mike

@tomasmatus
Copy link
Member Author

Hello @mac2net thanks for your interest to help me test this!
I'd recommend testing in a fedora-40 VM, maybe even spin up a new one just for this test.
Install cockpit using dnf and probably the developer group just to get build dependencies in.

dnf install cockpit
dnf groupinstall "Development Tools"
dnf install gettext nodejs npm make

pull and checkout to this branch

git clone https://github.com/tomasmatus/cockpit-files.git
cd cockpit-files
git switch mac-keybinds

and then you can follow README.md to build and install

make

make devel-install

Now you should have cockpit and cockpit-files ready so please connect to the webui using your macbook and try all of the keybinds listed in the help menu (the screenshot above). You might need to refresh the page if you had the webui running before installing cockpit-files.

@mac2net
Copy link

mac2net commented Oct 7, 2024

Hi @tomasmatus
Thanks for the instructions.
Before installing the developer version of Cockpit-files, is it good enough to do a normal DNF remove in order to delete the repository version of it?
Cheers Mike

@tomasmatus
Copy link
Member Author

@mac2net yes, if you already have cockpit-files installed from the repository you can remove it using dnf. Also when you want to uninstall the developer version there is make devel-uninstall.

@mac2net
Copy link

mac2net commented Oct 8, 2024

Hi @tomasmatus There are issues, mostly to do with visual feedback when navigating around. I compared it with the experience in XFCE accessed via VNC with a Mac app called Jump. There are some issues there too.

I don't do a lot of keyboard stuff inside a web page on the Mac, but the MacOS Finder UI is ingrained in my DNA. If you don't have a Mac you should take a trip to an Apple Store to test this. While it is probably not possible or even desired to reproduce the behaviour of the Finder, it is the starting point for Mac users with Cockpit Files.

There is one conflict with Safari – Command-Shift-L opens the left Tab Manager sidebar and in Firefox nothing happens, probably not allowed. Via VNC Control Shift L works fine

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

Successfully merging this pull request may close these issues.

shortcuts: Mac-native keyboard shortcuts
4 participants