New TextBox & IME integration tracking issue (report new TextBox issues here!)

[cmyr]

When #1636 lands, how druid handles text will change fundamentally, and there are almost certainly some issues that we will encounter as we start using the new implementation.

This issue is intended as a tracking/discussion issue for various problems with the new text work.

polish

There are a number of outstanding fixups I would like to land:

• [x] unify selection & movement types between druid-shell and druid
• [x] TextBox without focus should use an 'inactive' color for selections.
• [x] restructure druid::text module, centralizing all text-related types here
• [x] put TextBox Insets in the env
• [ ] handle more keys when 'simulating input' on windows/linux
• [ ] support more movement types (pageup/pagedown, e.g)
• [x] better implementation of 'select word' and 'select line': these don't currently behave well when clicks occur on boundaries.
• [x] preserve double/triple-click state through to drag gestures: drag after double-click should select at "word granularity", after triple click at "line granularity"

features

Larger related projects it might be nice to do while we're in here

• [x] select-all, copy, paste (and undo/redo, eventually) should work without the menus present (#1030)
• [ ] 'submit on return': there should be a mechanism for hooking the return key in order to perform some custom action.
• [ ] allow toggling selectable & editable flags: you may want a text field that can be selected but not edited, or a text field that cannot be edited or selected
• [ ] TextBox should have built-in support for undo/redo
• [ ] A mechanism for letting the user submit edit Actions.

bugs

• [x] move to start of line / end of line behaviour is incorrect
• [x] selections that include lines with emergency breaks (breaks that are not at whitespace) are not drawn correctly: https://github.com/linebender/piet/pull/412 ~- [ ] IME composing region is drawn incorrectly sometimes~ can't reproduce
• [x] IME composing window can draw at incorrect position ~- [ ] line height calculations are incorrect for some scripts (japanese)~ out of scope for this work

[jpochyla]

Thank you, it's very impressive!

• I think a lot of people were previously having custom controllers on TextBoxes, dispatching EditActions on shortcuts. This would be now done via textbox.text().input_handler().acquire().handle_action(), correct?
• TEXTBOX_INSETS is a constant again! :(

[madbonkey]

'submit on return': there should be a mechanism for hooking the return key in order to perform some custom action.

To be honest, I feel this is out of scope for text handling. What "submitting" or "commiting" means in regard to changes to state highly depends on the application and the control/inout type. You might think search boxes and go 'yeah that seems nice to have', but in reality the search might automatically "commit" after a debounce, or it's also committed by an icon next to it. It might require entirely different ways of "submitting" the input depending on devices and peripherals.

I feel like a mostly unrelated, general feature for forms/submission of inputs would be a more appropriate answer for this than cramming a special cased key handler into text widgets.

I would take a look at how the web does it; in the context of a <form> controls will trigger the form's submit handler. Different form controls have different modes of submission (i.e. a <button type="submit"> will submit its outer form, type="button" won't, a <textarea> won't submit on enter - this is different from <input>). Put another way, the submission logic is at the form level, while the controls only trigger a state change/commit (those change events are another very interesting topic).

In component-oriented web development (React etc.) you end up building forms out in a nestable manner, so that you can embed a full form as a nasted "control type" if you will, inside another form, or generally build complex controls for complex datastructures. What you need for this is a simple interface for a "form control", and a grouping/aggregation mechanism for change propagation and submission (both directions in the tree, so to speak).

[cmyr]

@jpochyla

• I think a lot of people were previously having custom controllers on TextBoxes, dispatching EditActions on shortcuts. This would be now done via textbox.text().input_handler().acquire().handle_action(), correct?

Good note, I'll add some proper API for this. It's tricky to get right, and needs a separate mechanism.

• TEXTBOX_INSETS is a constant again! :(

😢 I was hoping nobody would notice. Can look into improving this, I think I needed it as a const to make it work with Padding.

@madbonkey

'submit on return': there should be a mechanism for hooking the return key in order to perform some custom action.

To be honest, I feel this is out of scope for text handling. What "submitting" or "commiting" means in regard to changes to state highly depends on the application and the control/inout type. You might think search boxes and go 'yeah that seems nice to have', but in reality the search might automatically "commit" after a debounce, or it's also committed by an icon next to it. It might require entirely different ways of "submitting" the input depending on devices and peripherals.

I feel like a mostly unrelated, general feature for forms/submission of inputs would be a more appropriate answer for this than cramming a special cased key handler into text widgets.

I would take a look at how the web does it; in the context of a <form> controls will trigger the form's submit handler. Different form controls have different modes of submission (i.e. a <button type="submit"> will submit its outer form, type="button" won't, a <textarea> won't submit on enter - this is different from <input>). Put another way, the submission logic is at the form level, while the controls only trigger a state change/commit (those change events are another very interesting topic).

In component-oriented web development (React etc.) you end up building forms out in a nestable manner, so that you can embed a full form as a nasted "control type" if you will, inside another form, or generally build complex controls for complex datastructures. What you need for this is a simple interface for a "form control", and a grouping/aggregation mechanism for change propagation and submission (both directions in the tree, so to speak).

I think we're more or less on the same page. The problem here is that if the textbox has focus, then the textbox receives the return event, and it has to decide what to do with it. The user should be able to specify that they want to handle return themselves, but we don't want them to just handle the raw KeyEvent because if they do this while the IME is composing they will end up swallowing actions that the user-user thinks are just interactions with the IME.

So all I mean here is that we need to have some way for the user to configure the textbox to communicate to the user when the user should handle an enter/return event. This will be a simple mechanism, probably just a notification, but I could also imagine a custom Command that the user could handle in a controller.

[cmyr]

@jpochyla what edit actions were you actually interested in dispatching? If you have the ability to view and modify the text and the selection, is that enough?

[jpochyla]

@cmyr I was emulating basic Emacs shortcuts - C-A, C-E, C-K, etc., together with some shift variants. That is cursor movement, selection, text editing. I was hoping it would be possible to re-use the internal Action dispatching for this, but I probably don't understand the IME constraints here.

[HoNile]

I notice a new issue, if the Texbox is not big enough to print its content the cursor is drawn outside from the Texbox.

[cmyr]

@HoNile Can you provide a reproduction? Also would be good to know what platform you're on. :)

[HoNile]

@cmyr Yes of course I am on windows 10 and behind a reproduction code, just put the cursor at the end from a textbox and resize (horizontally) to a smaller size the windows.

use std::sync::Arc;

use druid::widget::{Flex, SizedBox, TextBox};
use druid::{AppLauncher, Data, Lens, LocalizedString, Widget, WidgetExt, WindowDesc};

const WINDOW_TITLE: LocalizedString<AppState> = LocalizedString::new("Text Options");

#[derive(Clone, Data, Lens)]
struct AppState {
    text: Arc<String>,
}

pub fn main() {
    let main_window = WindowDesc::new(build_root_widget())
        .title(WINDOW_TITLE)
        .window_size((300.0, 200.0));

    let initial_state = AppState {
        text: "blabla".to_string().into(),
    };

    AppLauncher::with_window(main_window)
        .launch(initial_state)
        .expect("Failed to launch application");
}

fn build_root_widget() -> impl Widget<AppState> {
    Flex::row()
        .with_flex_child(TextBox::multiline().expand_width().lens(AppState::text), 1.0)
        .with_flex_child(TextBox::new().expand_width().lens(AppState::text), 1.0)
        .with_child(SizedBox::empty().fix_width(110.0))
}

Edit: small clarification.

[cmyr]

Does the cursor stay outside of the textbox when you start typing again? I see this on macOS but it looks like a drawing artifact (the cursor doesn't blink, and it goes away when you next type)

[HoNile]

Yes the cursor stay outside when i type again, but the cursor doesn't blink when outside (unless I'm resizing the window).

Edit: I can even have two cursors if while an outside cursor is drawn I go back to the start from the texbox with arrow key.

Edit²: I can move the outside cursor from one character with arrow key on the single line textbox.

[ijijn]

Wow, these latest developments are all very intriguing! 🍿

Hope this is the right place to report... I'm getting a Druid panic every time I've changed the contents of a TextBox and then delete a parent widget. In my application, I'm switching display modes and destroying (setting to None) part of the optional widget tree when the following happens:

thread 'main' panicked at 'called Option::unwrap() on a None value'

in this function in window.rs

  pub(crate) fn get_ime_handler(
        &mut self,
        req_token: TextFieldToken,
        mutable: bool,
    ) -> Box<dyn InputHandler> {
        self.ime_handlers
            .iter()
            .find(|(token, _)| req_token == *token)
            .and_then(|(_, reg)| reg.document.acquire(mutable))
            .unwrap()
    }

The last commit that worked for me was 9c36187. I couldn't get 6e130df to build and d125769 onwards exhibits this panic. I'm getting this on Windows 10.

Any thoughts or suggestions would be most welcome. Thanks!

[cmyr]

Are you calling ctx.children_changed() after removing the widgets?

(this was previously only required when adding new widgets, but I think now we might need to always call it, in case a textbox was removed? 🤔)

[ijijn]

Funnily enough, this was one place I wasn't! Thanks for the catch... However, after adding the call to ctx.children_changed() back in, I'm still getting the same problem. 🤔

[cmyr]

okay cool, just wanted to rule that out, I'll look into this a bit more. Is this on macOS?

[ijijn]

Thank you! This is on Windows 10.

[Aphrodit]

I notice a new issue, the IME's window position is not on the TextBox. OS : Windows 10

[runiq]

OS : Windows 10

I just though I'd ask—the changelog said that #1636 is supposedly an implementation for IME support on MacOS. Since @Aphrodit has been testing this on Windows, does this mean that it can (should?) be tested on Linux as well?

[cmyr]

@runiq IME isn't implemented on windows or linux yet, but the changes in this patch will change behaviour on linux and windows; in particular some keys are not going to be handled correctly, so testing and reporting of those problems would definitely be helpful!

[kud1ing]

select-all, copy, paste (and undo/redo, eventually) should work

Observation on macOS with the widget_gallery:

• ✓ Cmd+C/V work for copying/pasting.
• ❌Cmd+A does not select the text

[cmyr]

@kud1ing thanks, yea, select-all isn't implemented yet, and nor are undo/redo.

[kud1ing]

@cmyr Thanks. Is much missing? The cmd does not look too bad ("druid-builtin.menu-select-all").

[Lnstree]

textbox inner show context_menu, the mouse cursor style should change Cursor::Arrow，but now it's style is Cursor::IBeam

[holykol]

It seems like #1425 continues to reproduce in a new window. Take a look at this example. Pasting some text and typing anything crashes the window.

use druid::{widget::*, *};

#[derive(Data, Lens, Clone)]
struct State {
    s: String,
}

pub fn main() {
    let main = Button::new("launch").on_click(launch);

    AppLauncher::with_window(WindowDesc::new(main))
        .use_simple_logger()
        .launch(())
        .unwrap();
}

fn launch(ctx: &mut EventCtx, data: &mut (), env: &Env) {
    ctx.new_sub_window(
        WindowConfig::default(),
        build_another(),
        State { s: "".to_owned() },
        env.clone(),
    );
}

fn build_another() -> impl Widget<State> {
    Flex::column().with_child(TextBox::new().with_placeholder("text").lens(State::s))
}

@cmyr

[cmyr]

I'm not able to trivially reproduce this on macOS? it's very possible I'm overlooking something.

I'd also discourage using new_sub_window unless you are trying to build a modal or similar; if you just want a new application window you should follow the pattern in the multiwin example.

[holykol]

I'm not able to trivially reproduce this on macOS? it's very possible I'm overlooking something.

I'm on Linux + Wayland, so it might be platform specific. Here's a little clip of me copying and pasting : https://user-images.githubusercontent.com/22733656/122043229-a299ac00-cdca-11eb-8cea-4779fdc63098.mp4

I'd also discourage using new_sub_window unless you are trying to build a modal or similar; if you just want a new application window you should follow the pattern in the multiwin example.

I guess you can call it a modal. Will take a look anyway. Thanks.

[kud1ing]

Regarding Ctrl/Cmd+A, see also https://github.com/linebender/druid/issues/1854

[Kethku]

I've been working on a todo app using druid and have generally been enjoying the experience, but recently ran into a serialization bug (fixed here: https://github.com/linebender/druid/pull/1871) which required me to use main. Sadly I pretty immediately ran into a panic that I think is related to the new textbox.

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid\src\window.rs:554:14
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099\/library\std\src\panicking.rs:515
   1: core::panicking::panic_fmt
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099\/library\core\src\panicking.rs:92
   2: core::panicking::panic
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099\/library\core\src\panicking.rs:50
   3: enum$<core::option::Option<alloc::boxed::Box<InputHandler, alloc::alloc::Global>>, 1, 18446744073709551615, Some>::unwrap<alloc::boxed::Box<InputHandler, alloc::alloc::Global>>
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099\library\core\src\option.rs:388
   4: druid::window::Window<tuple<kurbo::point::Point, im::vector::Vector<pando::widgets::todo::TodoItem>>>::get_ime_handler<tuple<kurbo::point::Point, im::vector::Vector<pando::widgets::todo::TodoItem>>>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid\src\window.rs:550
   5: druid::win_handler::Inner<tuple<kurbo::point::Point, im::vector::Vector<pando::widgets::todo::TodoItem>>>::get_ime_lock<tuple<kurbo::point::Point, im::vector::Vector<pando::widgets::todo::TodoItem>>>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid\src\win_handler.rs:514
   6: druid::win_handler::{{impl}}::acquire_input_lock<tuple<kurbo::point::Point, im::vector::Vector<pando::widgets::todo::TodoItem>>>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid\src\win_handler.rs:994
   7: druid_shell::text::simulate_input<WinHandler>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\text.rs:471
   8: druid_shell::backend::windows::window::{{impl}}::window_proc::{{closure}}::{{closure}}
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\window.rs:975
   9: druid_shell::backend::windows::window::MyWndProc::with_window_state<closure-0,bool>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\window.rs:466
  10: druid_shell::backend::windows::window::{{impl}}::window_proc::{{closure}}
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\window.rs:974
  11: druid_shell::backend::windows::window::{{impl}}::with_wnd_state::{{closure}}<closure-10,bool>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\window.rs:482
  12: enum$<core::option::Option<mut druid_shell::backend::windows::window::WndState*>, 1, 18446744073709551615, Some>::map<mut druid_shell::backend::windows::window::WndState*,bool,closure-0>
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099\library\core\src\option.rs:489
  13: druid_shell::backend::windows::window::MyWndProc::with_wnd_state<closure-10,bool>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\window.rs:482
  14: druid_shell::backend::windows::window::{{impl}}::window_proc
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\window.rs:963
  15: druid_shell::backend::windows::window::win_proc_dispatch
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\window.rs:1654
  16: CallWindowProcW
  17: DispatchMessageW
  18: druid_shell::backend::windows::application::Application::run
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\backend\windows\application.rs:159
  19: druid_shell::application::Application::run
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid-shell\src\application.rs:150
  20: druid::app::AppLauncher<tuple<kurbo::point::Point, im::vector::Vector<pando::widgets::todo::TodoItem>>>::launch<tuple<kurbo::point::Point, im::vector::Vector<pando::widgets::todo::TodoItem>>>
             at C:\Users\keith\.cargo\git\checkouts\druid-a3cf6f97328c9edc\38f37f1\druid\src\app.rs:265
  21: pando::main
             at .\src\main.rs:20
  22: core::ops::function::FnOnce::call_once<fn(),tuple<>>
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099\library\core\src\ops\function.rs:227

Unfortunately my app is a little convoluted, but basically I create a simple todo item with a textbox which edits the name of the todo using a lens. When the controller hears a key down for the return key, the todo item progresses to a new state, and the textbox is swapped out for a rawlabel. I'm not sure, but I think this causes a crash. I haven't dug in enough to determine what causes the crash but its on line https://github.com/linebender/druid/blob/master/druid/src/window.rs#L554

I'd really like to keep working on my app and exploring how far I can take it, but at this point I'm blocked between the serialization issue and this panic. Any tips or ideas for how to investigate this further would be appreciated.

[Kethku]

After some thinking, I decided to add a branch to my fork which introduces my PR fix to the current release (not containing the ime textbox changes) and am in a working state again. If time allows, I will try to create a minimal repro of the panic I ran into to help out here though.

[newcomb-luke]

I am here to report a new issue for TextBox. It is a very minor issue, but it seems like (on the current master branch at least) TextBox's text_pos (and therefore the text_position method on it) is actually never updated by layout() and will always be Point::ZERO

[Azorlogh]

Not sure if this is the same as one of the issues mentionned here, but I get a panic when a textbox gets deleted with a keypress. Minimal repro:

use druid::{widget::*, AppDelegate, *};

pub struct Closer;

impl<T: Data, W: Widget<T>> Controller<T, W> for Closer {
    fn event(&mut self, child: &mut W, ctx: &mut druid::EventCtx, event: &druid::Event, data: &mut T, env: &druid::Env) {
        match event {
            Event::KeyDown(evt) if evt.key == druid::keyboard_types::Key::Escape => ctx.submit_command(CLOSE),
            _ => child.event(ctx, event, data, env),
        }
    }
}

pub fn main() {
    let main = Flex::column()
        .with_child(
            Button::<Option<String>>::new("show textbox").on_click(|_, data: &mut _, _| {
                *data = Some(String::new());
            }),
        )
        .with_child(Maybe::new(
            || TextBox::new().controller(Closer),
            || SizedBox::empty(),
        ));

    AppLauncher::with_window(WindowDesc::new(main))
        .delegate(Delegate)
        .launch(None)
        .unwrap();
}

const CLOSE: Selector = Selector::new("close");

struct Delegate;

impl AppDelegate<Option<String>> for Delegate {
    fn command(&mut self, _ctx: &mut DelegateCtx, _target: Target, cmd: &Command, data: &mut Option<String>, _env: &Env) -> Handled {
        if cmd.is(CLOSE) {
            *data = None;
        }
        Handled::No
    }
}

The panic is a None getting unwrapped in the get_ime_handler method.

[kim8823]

A bit late to the party, but what about a different foreground color for selected text? Light themes could really benefit from this, otherwise you have to use a pretty light selection background color. Possibly even another color for the inactive case. I've poked through the code a bit and it seems the selection backgrounds are drawn completely separate from the text, so not sure if this is really possible/feasible. Just a suggestion.

Also might want to draw the inactive style when the entire window loses focus.