Start updating TextBox for piet text changes #1274
Conversation
cmyr
force-pushed
the
text-editor
branch
2 times, most recently
from
3f065ce
to
73cd9d3
Compare
this is largely a refactor; the idea is to separate out the logic of applying edits and managing selection state from the particular widget.
This is very preliminary; in particular there is no vertical scrolling. I think correctly handling scrolling should be a separate project, but I wanted to include a basic version of this to demonstrate that it is possible. Also: there's no vertical movement yet! Oops.
| EditAction::Move(mvmt) => self.selection = movement(mvmt, self.selection, data, false), | ||
| EditAction::ModifySelection(mvmt) => { | ||
| self.selection = movement(mvmt, self.selection, data, true) | ||
| } |
There was a problem hiding this comment.
I think you forgot EditAction::SelectAll?
| /// [`update`]: ../trait.Widget.html#tymethod.update | ||
| pub fn update(&mut self, ctx: &mut UpdateCtx, new_data: &T, env: &Env) { | ||
| if self.data_is_stale(new_data) { | ||
| self.layout.set_text(new_data.clone()); |
There was a problem hiding this comment.
Same here. If the textbox is not multi-lined, we should guard against new_data containing \n characters!
This issue can be seen when a non-multi lined textbox and another multi-lined textbox lens the same data!
There was a problem hiding this comment.
this is... a problem.
I'm not sure how best to think about this. Piet has no concept of a single-line layout, and trying to validate external data changes is difficult. It isn't clear what we should do when they happen.
I think my (bad) answer to this, for now, is that it is up to the programmer to ensure that newlines are not inserted externally. We could also definitely debug_panic if a newline exists in the initial text. 😒
|
Besides the delete button comment, everything here worked (on Windows). |
|
Thanks @justinmoon I'll take a look! |
This data must be TextStorage + EditableText, which means that *currently* it is only String, but just imagine what the future might hold? Discussion: there's still something funny going on with the TextStorage trait; this stuff as written isn't suitable for use with xi_rope, which was sort of the whole point.
There was a problem hiding this comment.
61a2c62 looks good. "select all" and "delete" both work now.
| /// selection state. | ||
| /// | ||
| /// [`EditableText`]: trait.EditableText.html | ||
| #[derive(Debug, Clone)] |
There was a problem hiding this comment.
I personally would appreciate a line of docstring what the type parameter is.
druid/src/text/editor.rs
Outdated
| self.selection = Selection::caret(self.selection.min() + text.len()); | ||
| } | ||
|
|
||
| /// Delete to previous grapheme if in caret mode. |
There was a problem hiding this comment.
This comment is imprecise: the backspace logic sometimes uses backspace granularity, but other times uses finer granularity to match expected behavior (individual vowels and diacritics in Indic scripts, harakat in Arabic, etc).
This is an attempt to integrate a bit of the new text stuff in to the
TextBoxwidget.Most of this code is a refactor that splits the actual manipulation of the buffer and selections into a new
Editorstruct that can be used to implement other text-editing widgets.Apart from that, there are two other commits; one of them changes
TextBoxfrom beingWidget<String>to beWidget<TextStorage + EditableText>, and the second adds aTextBox::multilineconstructor, as a demo of multiline editing.The multiline stuff is half-baked; the textbox does not support vertical scrolling. Ultimately it should be rewritten (maybe using @jneem's #1248 work?) so that it uses more standard scroll machinery; but this will be a bit fiddly, and I don't want to sink into it right now. I'll write up an issue detailing how I think this should work, though, and maybe someone can take it up?
This also includes a tweak to
examples/styled_text.rsto show a multiline texbox; I'm going to replace this with a simple markdown preview app, but that work is mostly done but it's on another computer, and will have to wait til I get to the studio (perhaps later today?)