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

Improve selection behaviour on focus change #1283

Merged
merged 1 commit into from
Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ You can find its changes [documented below](#060---2020-06-01).
- `LocalizedString` and `LabelText` use `ArcStr` instead of String ([#1245] by [@cmyr])
- `LensWrap` widget moved into widget module ([#1251] by [@cmyr])
- `Delegate::command` now returns `Handled`, not `bool` ([#1298] by [@jneem])
- `TextBox` selects all contents when tabbed to on macOS ([#1283] by [@cmyr])

### Deprecated

Expand Down Expand Up @@ -498,6 +499,7 @@ Last release without a changelog :(
[#1276]: https://github.com/linebender/druid/pull/1276
[#1278]: https://github.com/linebender/druid/pull/1278
[#1280]: https://github.com/linebender/druid/pull/1280
[#1283]: https://github.com/linebender/druid/pull/1283
[#1295]: https://github.com/linebender/druid/pull/1280
[#1298]: https://github.com/linebender/druid/pull/1298
[#1299]: https://github.com/linebender/druid/pull/1299
Expand Down
5 changes: 5 additions & 0 deletions druid/src/text/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ impl<T: TextStorage + EditableText> Editor<T> {
self.do_edit(EditAction::Paste(t), data)
}

/// Set the selection to the entire buffer.
pub fn select_all(&mut self, data: &T) {
self.selection = Selection::new(0, data.len());
}

fn mouse_action_for_event(&self, event: &MouseEvent) -> MouseAction {
let pos = self.layout.text_position_for_point(event.pos);
MouseAction {
Expand Down
7 changes: 0 additions & 7 deletions druid/src/text/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,6 @@ impl Selection {
self
}

/// Create a selection that starts at the beginning and ends at text length.
/// TODO: can text length be at a non-codepoint or a non-grapheme?
pub fn all(&mut self, text: &impl EditableText) {
self.start = 0;
self.end = text.len();
}

/// Create a caret, which is just a selection with the same and start and end.
pub fn caret(pos: usize) -> Self {
Selection {
Expand Down
27 changes: 25 additions & 2 deletions druid/src/widget/textbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ pub struct TextBox<T> {
cursor_timer: TimerToken,
cursor_on: bool,
multiline: bool,
/// true if a click event caused us to gain focus.
///
/// On macOS, if focus happens via click then we set the selection based
/// on the click position; if focus happens automatically (e.g. on tab)
/// then we select our entire contents.
was_focused_from_click: bool,
}

impl TextBox<()> {
Expand All @@ -66,6 +72,7 @@ impl<T> TextBox<T> {
cursor_on: false,
placeholder,
multiline: false,
was_focused_from_click: false,
}
}

Expand Down Expand Up @@ -196,6 +203,17 @@ impl<T: TextStorage + EditableText> TextBox<T> {
self.cursor_on = true;
self.cursor_timer = token;
}

// on macos we only draw the cursor if the selection is non-caret
#[cfg(target_os = "macos")]
fn should_draw_cursor(&self) -> bool {
self.cursor_on && self.editor.selection().is_caret()
}

#[cfg(not(target_os = "macos"))]
fn should_draw_cursor(&self) -> bool {
self.cursor_on
}
}

impl<T: TextStorage + EditableText> Widget<T> for TextBox<T> {
Expand All @@ -209,6 +227,7 @@ impl<T: TextStorage + EditableText> Widget<T> for TextBox<T> {
mouse.pos += Vec2::new(self.hscroll_offset, 0.0);

if !mouse.focus {
self.was_focused_from_click = true;
self.reset_cursor_blink(ctx.request_timer(CURSOR_BLINK_DURATION));
self.editor.click(&mouse, data);
}
Expand Down Expand Up @@ -284,7 +303,11 @@ impl<T: TextStorage + EditableText> Widget<T> for TextBox<T> {
self.editor.set_text(data.to_owned());
self.editor.rebuild_if_needed(ctx.text(), env);
}
LifeCycle::FocusChanged(_) => {
LifeCycle::FocusChanged(is_focused) => {
if cfg!(target_os = "macos") && *is_focused && !self.was_focused_from_click {
self.editor.select_all(data);
}
self.was_focused_from_click = false;
self.reset_cursor_blink(ctx.request_timer(CURSOR_BLINK_DURATION));
ctx.request_paint();
}
Expand Down Expand Up @@ -370,7 +393,7 @@ impl<T: TextStorage + EditableText> Widget<T> for TextBox<T> {
}

// Paint the cursor if focused and there's no selection
if is_focused && self.cursor_on {
if is_focused && self.should_draw_cursor() {
// the cursor position can extend past the edge of the layout
// (commonly when there is trailing whitespace) so we clamp it
// to the right edge.
Expand Down