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.

Sign up for GitHub

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

Jump to bottom

TextBox: handle SELECT_ALL #1931

Merged
merged 3 commits into from
Aug 23, 2021
Merged

TextBox: handle SELECT_ALL #1931

merged 3 commits into from
Aug 23, 2021

Conversation

maan2003
Copy link
Collaborator

Fixes #1854

cmyr
cmyr requested changes Aug 22, 2021
View reviewed changes
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

I don't think simulate_input is the right place to solve this, because iirc select_all is not necessarily something that is dispatched via IME.

To clarify: this is also currently broken on mac, even though mac does not use simulate input.

druid/src/widget/textbox.rs Outdated
@@ -363,7 +363,6 @@ impl<T: TextStorage + EditableText> TextBox<T> {
key if HotKey::new(SysMods::Cmd, "y").matches(key) && cfg!(windows) => {
Some(sys::REDO.to(our_id))
}
key if HotKey::new(SysMods::Cmd, "a").matches(key) => Some(sys::SELECT_ALL.to(our_id)),
Copy link
Member

Choose a reason for hiding this comment

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

hm, this should be working, so if it isn't it's probably that we just aren't handling this command? We'll need this anyway, since select-all can arrive from a menu (and hence a Command), and then it shouldn't be necessary to touch simulate_input.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, we are not handling SELECT_ALL.

@maan2003 maan2003 mentioned this pull request Aug 22, 2021
Merged
@maan2003 maan2003 changed the title simulate_input: Ctrl-A to SelectAll TextBox: handle SELECT_ALL Aug 22, 2021
@maan2003
Copy link
Collaborator Author

Makes sense, I didn't have opinion on where this should be handled, just wanted this to be fixed :)

cmyr
cmyr approved these changes Aug 23, 2021
View reviewed changes
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Cool, this looks right. Oddly there's still an issue with the selection not causing a paint pass (even with a rebase), but we can worry about that afterwards. :)

@maan2003 maan2003 merged commit 94674dc into linebender:master Aug 23, 2021
@maan2003 maan2003 deleted the simulate-input-select-all branch August 23, 2021 14:40
@maan2003
Copy link
Collaborator Author

still having that issue 🤕

@maan2003 maan2003 mentioned this pull request Nov 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmyr cmyr cmyr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ctrl+A doesn't select all text in a TextBox
2 participants
@maan2003 @cmyr