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

Implement vertical movement (for text editing) #1280

Merged
merged 1 commit into from
Oct 8, 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 @@ -46,6 +46,7 @@ You can find its changes [documented below](#060---2020-06-01).
- 'Tabs' widget allowing static and dynamic tabbed layouts. ([#1160] by [@rjwittams])
- `RichText` and `Attribute` types for creating rich text ([#1255] by [@cmyr])
- `request_timer` can now be called from `LayoutCtx` ([#1278] by [@Majora320])
- TextBox supports vertical movement ([#1280] by [@cmyr])

### Changed

Expand Down Expand Up @@ -490,6 +491,7 @@ Last release without a changelog :(
[#1255]: https://github.com/linebender/druid/pull/1255
[#1276]: https://github.com/linebender/druid/pull/1276
[#1278]: https://github.com/linebender/druid/pull/1278
[#1280]: https://github.com/linebender/druid/pull/1280

[Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master
[0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0
Expand Down
10 changes: 6 additions & 4 deletions druid/src/text/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,18 @@ impl<T: TextStorage + EditableText> Editor<T> {
EditAction::Delete => self.delete_forward(data),
EditAction::JumpDelete(mvmt) | EditAction::JumpBackspace(mvmt) => {
let to_delete = if self.selection.is_caret() {
movement(mvmt, self.selection, data, true)
movement(mvmt, self.selection, &self.layout, true)
} else {
self.selection
};
data.edit(to_delete.range(), "");
self.selection = Selection::caret(to_delete.min());
}
EditAction::Move(mvmt) => self.selection = movement(mvmt, self.selection, data, false),
EditAction::Move(mvmt) => {
self.selection = movement(mvmt, self.selection, &self.layout, false)
}
EditAction::ModifySelection(mvmt) => {
self.selection = movement(mvmt, self.selection, data, true)
self.selection = movement(mvmt, self.selection, &self.layout, true)
}
EditAction::Click(action) => {
if action.mods.shift() {
Expand Down Expand Up @@ -240,7 +242,7 @@ impl<T: TextStorage + EditableText> Editor<T> {

fn delete_forward(&mut self, data: &mut T) {
let to_delete = if self.selection.is_caret() {
movement(Movement::Right, self.selection, data, true)
movement(Movement::Right, self.selection, &self.layout, true)
} else {
self.selection
};
Expand Down
7 changes: 7 additions & 0 deletions druid/src/text/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@ impl<T: TextStorage> TextLayout<T> {
self.text.as_ref()
}

/// Returns the inner Piet [`TextLayout`] type.
///
/// [`TextLayout`]: ./piet/trait.TextLayout.html
pub fn layout(&self) -> Option<&PietTextLayout> {
self.layout.as_ref()
}

/// The size of the laid-out text.
///
/// This is not meaningful until [`rebuild_if_needed`] has been called.
Expand Down
94 changes: 77 additions & 17 deletions druid/src/text/movement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

//! Text editing movements.

use crate::text::{EditableText, Selection};
use crate::kurbo::Point;
use crate::piet::TextLayout as _;
use crate::text::{EditableText, Selection, TextLayout, TextStorage};

/// The specification of a movement.
#[derive(Debug, PartialEq, Clone, Copy)]
Expand All @@ -23,6 +25,10 @@ pub enum Movement {
Left,
/// Move to the right by one grapheme cluster.
Right,
/// Move up one visible line.
Up,
/// Move down one visible line.
Down,
/// Move to the left by one word.
LeftWord,
/// Move to the right by one word.
Expand All @@ -37,44 +43,98 @@ pub enum Movement {
EndOfDocument,
}

/// Compute the result of movement on a selection .
pub fn movement(m: Movement, s: Selection, text: &impl EditableText, modify: bool) -> Selection {
let offset = match m {
/// Compute the result of movement on a selection.
///
/// returns a new selection representing the state after the movement.
///
/// If `modify` is true, only the 'active' edge (the `end`) of the selection
/// should be changed; this is the case when the user moves with the shift
/// key pressed.
pub fn movement<T: EditableText + TextStorage>(
m: Movement,
s: Selection,
layout: &TextLayout<T>,
modify: bool,
) -> Selection {
let (text, layout) = match (layout.text(), layout.layout()) {
(Some(text), Some(layout)) => (text, layout),
_ => {
debug_assert!(false, "movement() called before layout rebuild");
return s;
}
};

let (offset, h_pos) = match m {
Movement::Left => {
if s.is_caret() || modify {
text.prev_grapheme_offset(s.end).unwrap_or(0)
text.prev_grapheme_offset(s.end)
.map(|off| (off, None))
.unwrap_or((0, s.h_pos))
} else {
s.min()
(s.min(), None)
}
}
Movement::Right => {
if s.is_caret() || modify {
text.next_grapheme_offset(s.end).unwrap_or(s.end)
text.next_grapheme_offset(s.end)
.map(|off| (off, None))
.unwrap_or((s.end, s.h_pos))
} else {
s.max()
(s.max(), None)
}
}

Movement::PrecedingLineBreak => text.preceding_line_break(s.end),
Movement::NextLineBreak => text.next_line_break(s.end),
Movement::Up => {
let cur_pos = layout.hit_test_text_position(s.end);
let h_pos = s.h_pos.unwrap_or(cur_pos.point.x);
if cur_pos.line == 0 {
(0, Some(h_pos))
} else {
let lm = layout.line_metric(cur_pos.line).unwrap();
let point_above = Point::new(h_pos, cur_pos.point.y - lm.height);
let up_pos = layout.hit_test_point(point_above);
(up_pos.idx, Some(point_above.x))
}
}
Movement::Down => {
let cur_pos = layout.hit_test_text_position(s.end);
let h_pos = s.h_pos.unwrap_or(cur_pos.point.x);
if cur_pos.line == layout.line_count() - 1 {
(text.len(), Some(h_pos))
} else {
let lm = layout.line_metric(cur_pos.line).unwrap();
// may not work correctly for point sizes below 1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

What I did in xi2 is add half the line height. I think that's most robust but I'm not sure it matters that much - point sizes below 1.0 are not really a thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

This comment was intended to be somewhat tongue in cheek 😅.

Wouldn't adding half the line-height would fail if our current line-height is more than 2x the height of the preceding line, which doesn't seem impossible?

This seemed like the most robust approach, but there may be issues I'm not considering.

let y_below = lm.y_offset + lm.height + 1.0;
let point_below = Point::new(h_pos, y_below);
let up_pos = layout.hit_test_point(point_below);
(up_pos.idx, Some(point_below.x))
}
}

Movement::StartOfDocument => 0,
Movement::EndOfDocument => text.len(),
Movement::PrecedingLineBreak => (text.preceding_line_break(s.end), None),
Movement::NextLineBreak => (text.next_line_break(s.end), None),

Movement::StartOfDocument => (0, None),
Movement::EndOfDocument => (text.len(), None),

Movement::LeftWord => {
if s.is_caret() || modify {
let offset = if s.is_caret() || modify {
text.prev_word_offset(s.end).unwrap_or(0)
} else {
s.min()
}
};
(offset, None)
}
Movement::RightWord => {
if s.is_caret() || modify {
let offset = if s.is_caret() || modify {
text.next_word_offset(s.end).unwrap_or(s.end)
} else {
s.max()
}
};
(offset, None)
}
};
Selection::new(if modify { s.start } else { offset }, offset)

let start = if modify { s.start } else { offset };
Selection::new(start, offset).with_h_pos(h_pos)
}
16 changes: 15 additions & 1 deletion druid/src/text/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,20 @@ pub struct Selection {

/// The active edge of a selection, as a byte offset.
pub end: usize,

/// The saved horizontal position, during vertical movement.
pub h_pos: Option<f64>,
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a little weird how this variable is called h_pos here and x_pos on 105

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree :)

}

impl Selection {
/// Create a selection that begins at start and goes to end.
/// Like dragging a mouse from start to end.
pub fn new(start: usize, end: usize) -> Self {
Selection { start, end }
Selection {
start,
end,
h_pos: None,
}
}

/// Create a selection that starts at the beginning and ends at text length.
Expand All @@ -49,9 +56,16 @@ impl Selection {
Selection {
start: pos,
end: pos,
h_pos: None,
}
}

/// Construct a new selection from this selection, with the provided h_pos.
pub fn with_h_pos(mut self, h_pos: Option<f64>) -> Self {
self.h_pos = h_pos;
self
}

/// If start == end, it's a caret
pub fn is_caret(self) -> bool {
self.start == self.end
Expand Down
12 changes: 12 additions & 0 deletions druid/src/text/text_input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,18 @@ impl TextInput for BasicTextInput {
k_e if (HotKey::new(None, KbKey::ArrowRight)).matches(k_e) => {
EditAction::Move(Movement::Right)
}
k_e if (HotKey::new(None, KbKey::ArrowUp)).matches(k_e) => {
EditAction::Move(Movement::Up)
}
k_e if (HotKey::new(None, KbKey::ArrowDown)).matches(k_e) => {
EditAction::Move(Movement::Down)
}
k_e if (HotKey::new(SysMods::Shift, KbKey::ArrowUp)).matches(k_e) => {
EditAction::ModifySelection(Movement::Up)
}
k_e if (HotKey::new(SysMods::Shift, KbKey::ArrowDown)).matches(k_e) => {
EditAction::ModifySelection(Movement::Down)
}
// Delete left word
k_e if (HotKey::new(SysMods::Cmd, KbKey::Backspace)).matches(k_e) => {
EditAction::JumpBackspace(Movement::LeftWord)
Expand Down