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

Request focus on widget #145

Open
Uriopass opened this issue Feb 14, 2024 · 4 comments
Open

Request focus on widget #145

Uriopass opened this issue Feb 14, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@Uriopass
Copy link
Contributor

Currently writing a simple chat. I'd like to be able to automatically focus the textbox when pressing "T" and the textbox appears.
There appears to be some state about which widget is focused as there is WidgetEvent::FocusChanged.

I was thinking of adding a method to the Dom with a widget id. Would that be a good API?

so you could do

let resp = textbox(...);
context::dom().request_focus(resp.id);

could even be implemented on top of Response

let resp = textbox(...);
resp.request_focus();
@LPGhatguy
Copy link
Member

Widgets can currently request focus by using ctx.input.set_selection(Some(id)) from an event. TextBox does this for an example. That API is a little bit clunky though and I don't think there's any way to access the input context from outside of a widget implementation.

I like both of the proposed APIs and I think they could work well together! The Dom::request_focus API could replace the current selection API.

This proposal would probably require us to either move some input state into the Dom struct itself, or to add some sort of pending requested focus change field to Input that we could flush at the end of an update. I'm not sure what would be the better call. State being split across several structs was an early yakui design decision and I don't know if it's one that we need to uphold very strongly.

@LPGhatguy LPGhatguy added the enhancement New feature or request label Feb 15, 2024
@Uriopass
Copy link
Contributor Author

Instead of a pending requested focus on Input, I think a pending requested focus on Dom makes more sense as I feel this state belongs more to Input?

I've done that at #146

@Uriopass Uriopass reopened this Feb 15, 2024
@HexyWitch
Copy link
Contributor

HexyWitch commented Aug 2, 2024

I'd like to bring back the idea of handling this in Input. I propose adding: Event::RequestFocus(Option<WidgetId>)

The reason for this is that it would be useful to be able to sequentially request focus then send input events to the focused widget. For instance

Move cursor to the end of an input

yak.handle_event(Event::RequestFocus(Some(chat_widget)));
// or: `yak.request_focus(Some(chat_widget));`
yak.handle_event(Event::KeyChanged{ key: KeyCode::End, down: true});
yak.handle_event(Event::KeyChanged{ key: KeyCode::End, down: false});

Activate multiple text inputs

for textbox in textboxes {
    yak.handle_event(Event::RequestFocus(Some(textbox)));
    // or: `yak.request_focus(Some(textbox));`
    yak.handle_event(Event::KeyChanged{ key: KeyCode::Enter, down: true});
    yak.handle_event(Event::KeyChanged{ key: KeyCode::Enter, down: false});
}
yak.handle_event(Event::RequestFocus(None));
// or: `yak.request_focus(None);`

There could be a Yakui::request_focus helper:

fn request_focus(&mut self, widget_id: Option<WidgetId>) {
    self.handle_event(Event::RequestFocus(widget_id));
}

But also feeding fake input events to yakui to do things that maybe should be supported through their own APIs is maybe a huge hack. So I understand if that's not really desirable.

The behavior of the current dom API is also a little bit subtle. You might use it like:

// Doctor, it hurts when I do this
yak.dom().request_focus(chat_input);
yak.handle_event(Event::KeyInput{ key: KeyCode::End, bool: true});
yak.handle_event(Event::KeyInput{ key: KeyCode::End, bool: false});

// Doctor: have you tried not doing that

and expect these events to happen sequentially, but the acutal widget focus will happen only on the next dom rebuild.

Moving the api from Dom into Yakui makes the behavior more predictable, and makes Dom entirely agnostic of InputState (the only reason Dom::finish(&self, input: &InputState) takes InputState is to set the selection if requested through the Dom.

This breaks the Response::request_focus API, but I don't think that's a bad thing. I'm not sure that API is well motivated. You can instead grab the WidgetId from the response and request focus separately, and it will work the same and be clearer with no deferred effect.

Summary

I don't think Dom was the right place to handle this. I think the mechanics of focusing should be encapsulated to InputState and that the helper API belongs in Yakui.

@msparkles
Copy link
Contributor

msparkles commented Sep 22, 2024

Sometimes we think about custom events in yakui that are local to widgets. We don't personally believe that, as a UI library focused on providing you with scaffolding to build your own, and not providing a fully fleshed out UI, we should aim to provide this stuff instead.

There's an idea to take the request_focus stuff and think about it in another way: Exclusivity.

There's a lot of ways to do this, maybe something like a Set(&'static str) and Unset(&'static str) event, to dispatch Unset("focus") to everything and then dispatch Set("focus") to one widget. But then there'd be an issue with the locality of those events.

An idea is to somehow leverage Rust's type system and perhaps use a typemap or something, and then you can import common events like Focus.

This can be nice because you can stuff all the collected events into the Yakui state, move it and pass it around in all the widgets as they handle them. Perhaps one would also need to use the widget Id to use this in situations like focusing a single widget.

Any thoughts? We'd like to discuss 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants