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

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

Open
10 of 16 tasks
cmyr opened this issue Mar 16, 2021 · 31 comments
Open
10 of 16 tasks
Labels
text widget concerns a particular widget

Comments

@cmyr
Copy link
Member

cmyr commented Mar 16, 2021

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:

  • unify selection & movement types between druid-shell and druid
  • TextBox without focus should use an 'inactive' color for selections.
  • restructure druid::text module, centralizing all text-related types here
  • put TextBox Insets in the env
  • handle more keys when 'simulating input' on windows/linux
  • support more movement types (pageup/pagedown, e.g)
  • better implementation of 'select word' and 'select line': these don't currently behave well when clicks occur on boundaries.
  • 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

  • select-all, copy, paste (and undo/redo, eventually) should work without the menus present (Copying and pasting in textbox does not work (at least on windows) #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

  • move to start of line / end of line behaviour is incorrect
  • selections that include lines with emergency breaks (breaks that are not at whitespace) are not drawn correctly: Fix incorrect selection rects when emergency breaking piet#412
    - [ ] IME composing region is drawn incorrectly sometimes can't reproduce
  • IME composing window can draw at incorrect position
    - [ ] line height calculations are incorrect for some scripts (japanese) out of scope for this work
@jpochyla
Copy link
Contributor

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
Copy link

'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
Copy link
Member Author

cmyr commented Mar 17, 2021

@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 added a commit that referenced this issue Mar 17, 2021
The textbox will now receive copy/cut/paste/undo/redo/select-all
without a menu being present.

Several of these (select-all, undo/redo) do not currently have
implementations, but we will at least get the commands.

- progress on #1652
- closes #1030
cmyr added a commit that referenced this issue Mar 17, 2021
The textbox will now receive copy/cut/paste/undo/redo/select-all
without a menu being present.

Several of these (select-all, undo/redo) do not currently have
implementations, but we will at least get the commands.

- progress on #1652
- closes #1030
cmyr added a commit that referenced this issue Mar 17, 2021
The textbox will now receive copy/cut/paste/undo/redo/select-all
without a menu being present.

Several of these (select-all, undo/redo) do not currently have
implementations, but we will at least get the commands.

- progress on #1652
- closes #1030
cmyr added a commit that referenced this issue Mar 18, 2021
@cmyr
Copy link
Member Author

cmyr commented Mar 18, 2021

@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
Copy link
Contributor

@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.

cmyr added a commit that referenced this issue Mar 18, 2021
This is particularly relevant for when the user double-clicks a word.
Previously if the click fell on a word boundary we would not
recognize that; now if the click falls on a word boundary we will
treat that as the start of the new selection range.

In addition, word select now selects *anything* between two word
boundaries; we do not care if it is actually a word. As an example:
if the user double clicks in whitespcae, we will select any
contiguous whitespace.

progress on #1652
@HoNile
Copy link
Contributor

HoNile commented Mar 18, 2021

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
Copy link
Member Author

cmyr commented Mar 18, 2021

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

cmyr added a commit that referenced this issue Mar 19, 2021
This is particularly relevant for when the user double-clicks a word.
Previously if the click fell on a word boundary we would not
recognize that; now if the click falls on a word boundary we will
treat that as the start of the new selection range.

In addition, word select now selects *anything* between two word
boundaries; we do not care if it is actually a word. As an example:
if the user double clicks in whitespcae, we will select any
contiguous whitespace.

progress on #1652
@HoNile
Copy link
Contributor

HoNile commented Mar 19, 2021

@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
Copy link
Member Author

cmyr commented Mar 19, 2021

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)

cmyr added a commit that referenced this issue Mar 19, 2021
This is particularly relevant for when the user double-clicks a word.
Previously if the click fell on a word boundary we would not
recognize that; now if the click falls on a word boundary we will
treat that as the start of the new selection range.

In addition, word select now selects *anything* between two word
boundaries; we do not care if it is actually a word. As an example:
if the user double clicks in whitespcae, we will select any
contiguous whitespace.

progress on #1652
@HoNile
Copy link
Contributor

HoNile commented Mar 19, 2021

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.

cmyr added a commit that referenced this issue Mar 19, 2021
This is particularly relevant for when the user double-clicks a word.
Previously if the click fell on a word boundary we would not
recognize that; now if the click falls on a word boundary we will
treat that as the start of the new selection range.

In addition, word select now selects *anything* between two word
boundaries; we do not care if it is actually a word. As an example:
if the user double clicks in whitespcae, we will select any
contiguous whitespace.

progress on #1652
@ijijn
Copy link

ijijn commented Mar 19, 2021

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
Copy link
Member Author

cmyr commented Mar 19, 2021

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
Copy link

ijijn commented Mar 19, 2021

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
Copy link
Member Author

cmyr commented Mar 19, 2021

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

@ijijn
Copy link

ijijn commented Mar 19, 2021

Thank you! This is on Windows 10.

@Aphrodit
Copy link

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

@runiq
Copy link

runiq commented Mar 22, 2021

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
Copy link
Member Author

cmyr commented Mar 22, 2021

@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!

cmyr added a commit that referenced this issue Mar 29, 2021
@cmyr cmyr mentioned this issue Mar 30, 2021
cmyr added a commit that referenced this issue Mar 30, 2021
The textbox will now receive copy/cut/paste/undo/redo/select-all
without a menu being present.

Several of these (select-all, undo/redo) do not currently have
implementations, but we will at least get the commands.

- progress on #1652
- closes #1030
cmyr added a commit that referenced this issue Mar 30, 2021
The textbox will now receive copy/cut/paste/undo/redo/select-all
without a menu being present.

Several of these (select-all, undo/redo) do not currently have
implementations, but we will at least get the commands.

- progress on #1652
- closes #1030
richard-uk1 pushed a commit to richard-uk1/druid that referenced this issue Apr 6, 2021
This is particularly relevant for when the user double-clicks a word.
Previously if the click fell on a word boundary we would not
recognize that; now if the click falls on a word boundary we will
treat that as the start of the new selection range.

In addition, word select now selects *anything* between two word
boundaries; we do not care if it is actually a word. As an example:
if the user double clicks in whitespcae, we will select any
contiguous whitespace.

progress on linebender#1652
richard-uk1 pushed a commit to richard-uk1/druid that referenced this issue Apr 6, 2021
richard-uk1 pushed a commit to richard-uk1/druid that referenced this issue Apr 6, 2021
The textbox will now receive copy/cut/paste/undo/redo/select-all
without a menu being present.

Several of these (select-all, undo/redo) do not currently have
implementations, but we will at least get the commands.

- progress on linebender#1652
- closes linebender#1030
@kud1ing
Copy link
Contributor

kud1ing commented Apr 8, 2021

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
Copy link
Member Author

cmyr commented Apr 11, 2021

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

@kud1ing
Copy link
Contributor

kud1ing commented May 4, 2021

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

Bildschirmfoto 2021-05-04 um 11 51 32

@Lnstree
Copy link

Lnstree commented May 27, 2021

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

@holykol
Copy link

holykol commented Jun 14, 2021

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
Copy link
Member Author

cmyr commented Jun 14, 2021

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
Copy link

holykol commented Jun 15, 2021

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
Copy link
Contributor

kud1ing commented Jul 6, 2021

Regarding Ctrl/Cmd+A, see also #1854

@Kethku
Copy link
Contributor

Kethku commented Jul 12, 2021

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: #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
Copy link
Contributor

Kethku commented Jul 12, 2021

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

kim8823 commented Feb 19, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
text widget concerns a particular widget
Projects
None yet
Development

No branches or pull requests