marc2332 / freya

Cross-platform GUI library for 🦀 Rust powered by 🧬 Dioxus and 🎨 Skia.
https://freyaui.dev/
MIT License
1.42k stars 56 forks source link

input: Failure when reducing text string while editing #348

Closed katyo closed 11 months ago

katyo commented 11 months ago

I remove some characters while user input so the length of edited text is reduced but cursor position stay some as before:

thread 'main' panicked at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ropey-1.6.1/src/rope.rs:345:41:
called `Result::unwrap()` on an `Err` value: Char index out of bounds: char index 2, Rope/RopeSlice char length 1
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: core::result::unwrap_failed
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/result.rs:1652:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/result.rs:1077:23
   4: ropey::rope::Rope::insert
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ropey-1.6.1/src/rope.rs:345:9
   5: <freya_hooks::rope_editor::RopeEditor as freya_hooks::text_editor::TextEditor>::insert
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/hooks/src/rope_editor.rs:50:9
   6: freya_hooks::text_editor::TextEditor::process_key
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/hooks/src/text_editor.rs:367:33
   7: freya_hooks::use_editable::UseEditable::process_event::{{closure}}
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/hooks/src/use_editable.rs:111:33
   8: dioxus_hooks::usestate::UseState<T>::with_mut
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-hooks-0.4.0/src/usestate.rs:248:9
   9: freya_hooks::use_editable::UseEditable::process_event
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/hooks/src/use_editable.rs:110:17
  10: freya_components::input::Input::{{closure}}
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/components/src/input.rs:110:17
  11: dioxus_core::scopes::ScopeState::listener::{{closure}}
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-core-0.4.2/src/scopes.rs:532:21
  12: dioxus_core::virtual_dom::VirtualDom::handle_event
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dioxus-core-0.4.2/src/virtual_dom.rs:422:37
  13: freya_renderer::app::App<State>::poll_vdom::{{closure}}
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/renderer/src/app.rs:181:33
  14: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/future/future.rs:125:9
  15: futures_util::future::future::FutureExt::poll_unpin
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.28/src/future/future/mod.rs:562:9
  16: freya_renderer::app::App<State>::poll_vdom
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/renderer/src/app.rs:191:23
  17: freya_renderer::event_loop::run_event_loop::{{closure}}
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/renderer/src/event_loop.rs:80:21
  18: winit::platform_impl::platform::sticky_exit_callback
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/mod.rs:884:9
  19: winit::platform_impl::platform::x11::EventLoop<T>::run_return::single_iteration
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/x11/mod.rs:348:21
  20: winit::platform_impl::platform::x11::EventLoop<T>::run_return
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/x11/mod.rs:483:27
  21: winit::platform_impl::platform::x11::EventLoop<T>::run
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/x11/mod.rs:498:25
  22: winit::platform_impl::platform::EventLoop<T>::run
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/platform_impl/linux/mod.rs:792:56
  23: winit::event_loop::EventLoop<T>::run
             at /home/kayo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.28.7/src/event_loop.rs:305:9
  24: freya_renderer::event_loop::run_event_loop
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/renderer/src/event_loop.rs:38:5
  25: freya_renderer::run_app
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/renderer/src/lib.rs:64:5
  26: freya::launch::launch_cfg
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/freya/src/launch.rs:214:5
  27: freya::launch::launch
             at /home/kayo/.cargo/git/checkouts/freya-aa88117f1f713ee2/6dda57e/crates/freya/src/launch.rs:33:5
  28: ex1::main
             at ./src/main.rs:9:5
  29: core::ops::function::FnOnce::call_once
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/ops/function.rs:250:5

I think we need sync cursor position with actual text string length before inserting or removing characters while editing to prevent such wrong behavior. Currently I'm not quite familiar with dioxus/freya architecture so I don't sure where the right place to do it.

katyo commented 11 months ago

I fixed it in #349. It works fine but I'm not sure that my solution is fully correct.

marc2332 commented 11 months ago

Hey! Your fix makes sense but I can't seem to reproduce the error without your fix. What exactly did you do to trigger it?

katyo commented 11 months ago

My simplified example to reproduce error:

let text = use_state(cx, || "".to_string());

Input {
  value: text.get().clone(),
  onchange: |event: String| text.set(event.trim().to_string()),
}

In empty input press space next insert some non-space char and either press space again or backspace.

marc2332 commented 11 months ago

Your fix indeed seems to fix it! Although now I see another issue, when you press space, it will move the cursor even though the text doesn't contain a space, I'll see what I can do about that

katyo commented 11 months ago

Looks like we need a way to precise control input which includes updating cursor position (and maybe text selection) simultaneously with updating contents.

marc2332 commented 11 months ago

Looks like we need a way to precise control input which includes updating cursor position (and maybe text selection) simultaneously with updating contents.

No worries! I just pushed a fix in your PR, I think it works great, let me know what you think!

katyo commented 11 months ago

@marc2332 I tested your changes. Now it works as expected. Thank you so much! Have you future plans to add ability to control text cursor and selection?

Also I found some wrong behavior related to text selection. I will create new issue for that.

marc2332 commented 11 months ago

Have you future plans to add ability to control text cursor and selection?

Of course! Many things can still be improved