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

Keymap code cleanup #989

Open
juliusknorr opened this issue Aug 26, 2020 · 3 comments
Open

Keymap code cleanup #989

juliusknorr opened this issue Aug 26, 2020 · 3 comments

Comments

@juliusknorr
Copy link
Member

🧐 Minor nitpick: Keymap class sounds very generic and behaves generic in that all of it's behaviour is specified through the options during construction. At the same time it now implements special treatment for ctrl-f. Seems counter intuitive that by adding a keymap to the editor that handles mod-s you also get special treatment for ctrl-f.

Maybe put the plugin into a separate class like BrowserSearch or so?

Originally posted by @azul in #988 (comment)

@luka-nextcloud
Copy link
Contributor

It's still valid.

@mejo-
Copy link
Member

mejo- commented Nov 9, 2022

Mhhh, so this is merely about renaming a javascript class? 🧐

I'd suggest to either fix it straight away or close the issue 😉

I don't think there's much value in having issues about minor refactoring. Either it will happen along with other work anyway (and this issue will not be memorized by then) or it doesn't seem to be important enough 🤪

@juliusknorr
Copy link
Member Author

Agreed, I think Keymap still sounds sane as a naming, but we should move the Ctrl+S registration from the Editor.vue file to the actual plugin instead of using the options.

@luka-nextcloud Mind to have a look into that?

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

No branches or pull requests

3 participants