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 CTRL + Backspace for deleting word in Entry #4084

Closed
wants to merge 5 commits into from

Conversation

Jacalz
Copy link
Member

@Jacalz Jacalz commented Jul 22, 2023

Description:

This adds support for pressing ctrl + backspace for deleting a whole word.
Opening as draft until I have added support for ctrl + delete for deleting a whole word to the left as well.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

@Jacalz Jacalz changed the title Entry backspace+ctrl Support CTRL + Backspace for deleting word in Entry Jul 22, 2023
widget/entry.go Outdated
e.shortcut.AddShortcut(&desktop.CustomShortcut{KeyName: fyne.KeyLeft, Modifier: fyne.KeyModifierShortcutDefault}, unselectMoveWord)
e.shortcut.AddShortcut(&desktop.CustomShortcut{KeyName: fyne.KeyLeft, Modifier: fyne.KeyModifierShortcutDefault | fyne.KeyModifierShift}, selectMoveWord)
e.shortcut.AddShortcut(&desktop.CustomShortcut{KeyName: fyne.KeyRight, Modifier: fyne.KeyModifierShortcutDefault}, unselectMoveWord)
e.shortcut.AddShortcut(&desktop.CustomShortcut{KeyName: fyne.KeyRight, Modifier: fyne.KeyModifierShortcutDefault | fyne.KeyModifierShift}, selectMoveWord)
e.shortcut.AddShortcut(&desktop.CustomShortcut{KeyName: fyne.KeyBackspace, Modifier: fyne.KeyModifierShortcutDefault}, removeWord)
Copy link
Contributor

Choose a reason for hiding this comment

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

On Mac the shortcut for this is Option (alt) + backspace. I think we should try to follow this convention, either using different files with build guards to define the shortcuts, or runtime.GOOS checks

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, interesting. I updated it to use the alt shortcut on macOS instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting find: It seems like Go is smart enough to optimize away runtime.GOOS checks. It is a trivial optimization but nevertheless, I am impressed :)
https://go.godbolt.org/z/b8GYaxz6Y

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

This is slick!
I know it's still draft but you added the other shortcut now so I wondered if that was just oversight.

@Jacalz
Copy link
Member Author

Jacalz commented Jul 23, 2023

Thanks but it is still draft because there seems to be a few bugs that I need to sort out :/

@dweymouth
Copy link
Contributor

I wonder if we could address #2462 in this PR (or a follow up) as well :)

@Jacalz
Copy link
Member Author

Jacalz commented Aug 2, 2023

Do we not already support that?

@dweymouth
Copy link
Contributor

No we don't, not yet. On Mac it's the same with Alt instead of Ctrl/Cmd being the modifier.

@Jacalz
Copy link
Member Author

Jacalz commented Aug 2, 2023

I suppose that issue should be closed and there should be a specific one for macOS using Alt key instead in that case. The idea of the original issue seems to be fixed (ctrl is supported). It doesn't necessarily seem like something that has to be part of this PR. You're more than welcome to follow up with a PR yourself. I only have very infrequent access to my macOS computer so can't verify the change ;)

@dweymouth
Copy link
Contributor

I suppose that issue should be closed and there should be a specific one for macOS using Alt key instead in that case. The idea of the original issue seems to be fixed (ctrl is supported). It doesn't necessarily seem like something that has to be part of this PR. You're more than welcome to follow up with a PR yourself. I only have very infrequent access to my macOS computer so can't verify the change ;)

Oh, my bad - the Ctrl+left/right was added on develop and I didn't realize it since it wasn't present in my app building against 2.3.x. All that would be needed to match MacOS conventions would be to use your deleteModifier value for all the word-navigation shortcuts instead of ShortcutDefault modifier (and rename it to editModifier or something more general?)

@Jacalz
Copy link
Member Author

Jacalz commented Aug 3, 2023

What I am trying to say is that this is quite broken at the moment and it will be a while until I get it fixed up. You might want to open a PR if you want it on v2.4.0.

@Jacalz
Copy link
Member Author

Jacalz commented Jan 7, 2024

I haven't had time to fix up the issues here. Closing for now and opening #4524 in case anyone wants to pick it up.

@Jacalz Jacalz closed this Jan 7, 2024
@Jacalz Jacalz deleted the entry-backspace+ctrl branch January 7, 2024 16:14
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.

3 participants