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

[feature request] A widget that sets the cursor on hover. #1478

Closed
richard-uk1 opened this issue Dec 20, 2020 · 10 comments
Closed

[feature request] A widget that sets the cursor on hover. #1478

richard-uk1 opened this issue Dec 20, 2020 · 10 comments
Labels
D-Easy just needs to be implemented enhancement adds or requests a new feature question causes uncertainty widget concerns a particular widget

Comments

@richard-uk1
Copy link
Collaborator

richard-uk1 commented Dec 20, 2020

Currently if you want to set the cursor for a widget you need to intercept events. This means using a custom Controller, which can be fiddly and error-prone.

This request involves adding a new widget (probably called Cursor) to druid::widget that sets the cursor whenever it is hovered, and a cursor method to WidgetExt that wraps the widget in a Cursor.

@richard-uk1 richard-uk1 added D-Easy just needs to be implemented help wanted has no one working on it yet widget concerns a particular widget enhancement adds or requests a new feature and removed help wanted has no one working on it yet labels Dec 20, 2020
@richard-uk1
Copy link
Collaborator Author

I'll wait for people to comment on whether it's a good idea before I say people should have a go at implementing it.

@richard-uk1
Copy link
Collaborator Author

richard-uk1 commented Dec 20, 2020

Here's a controller I made to do something like this. It's probably better to use a Controller rather than a Widget, but I'm not 100% sure this is correct:

/// Button controller.
///
/// Responsible for setting the cursor and calling a contained on_click function on click.
struct BtnController<T> {
    cursor: Cursor,
    on_click: Box<dyn Fn(&mut EventCtx, &mut T, &Env)>,
}

impl<T> BtnController<T> {
    fn new(cursor: Cursor, on_click: impl Fn(&mut EventCtx, &mut T, &Env) + 'static) -> Self {
        BtnController {
            cursor,
            on_click: Box::new(on_click),
        }
    }
}

impl<T, W: Widget<T>> Controller<T, W> for BtnController<T> {
    fn event(&mut self, child: &mut W, ctx: &mut EventCtx, event: &Event, data: &mut T, env: &Env) {
        match event {
            Event::MouseMove(_) => ctx.set_cursor(&self.cursor),
            Event::MouseDown(MouseEvent { buttons, .. }) if buttons.has_left() => {
                ctx.set_active(true)
            }
            Event::MouseUp(MouseEvent { buttons, .. }) if !buttons.has_left() => {
                if ctx.is_active() {
                    (*self.on_click)(ctx, data, env);
                    ctx.set_active(false);
                }
            }
            _ => (),
        }
    }
}

a Cursor widget wouldn't need the on_click callback, so would be simpler.

@jneem
Copy link
Collaborator

jneem commented Dec 21, 2020

Do the changes in #1433 help here? I think they should give more-or-less the behavior you want without adding wrapper widget...

@richard-uk1
Copy link
Collaborator Author

@jneem say I have a widget like Flex::row().with_child(Label::new("test")), how do I make the cursor change on hover?

@jneem
Copy link
Collaborator

jneem commented Dec 21, 2020

Oh, right, I was thinking of the case where you already have a custom widget.

What if we added set_cursor to the lifecycle context, and then added a on_added function to WidgetExt, allowing for a callback that's called when the widget gets WidgetAdded? I can imagine this being useful for other purposes, and it would allow you to do

Flex::row()
    .with_child(Label::new("test"))
    .on_added(|ctx, _data, _env| ctx.set_cursor(&Cursor::Arrow));

@richard-uk1
Copy link
Collaborator Author

richard-uk1 commented Dec 21, 2020

Hmm yeah that would work as well. So this is a custom controller that runs the callback on LifeCycle::WidgetAdded, with a helper function?

@jneem
Copy link
Collaborator

jneem commented Dec 21, 2020

That's what I was thinking, yes. If there are other things that are worth having callbacks for, they could be included as well, of course.

@richard-uk1
Copy link
Collaborator Author

richard-uk1 commented Dec 21, 2020

Cool, then if we wanted then we could add a WidgetExt::with_cursor function on top. Sounds like a (more general) plan.

@cmyr or @raphlinus what do you think about a WidgetExt function that allows you to do something on LifeCycle::WidgetAdded (called e.g. WidgetExt::on_added)? And how about a WidgetExt::with_cursor to set the cursor (a special case of WidgetExt::on_added)?

@richard-uk1 richard-uk1 added the question causes uncertainty label Dec 21, 2020
@cmyr
Copy link
Member

cmyr commented Dec 21, 2020

I think this makes a lot of sense!

(I think that on_added is a very strong idea, and i think on_cursor makes sense too, although it does seem like it can be implemented pretty easily as part of on_added so it doesn't feel like as clear a win. on_added feels like a big win!)

@richard-uk1
Copy link
Collaborator Author

richard-uk1 commented Dec 21, 2020

Ok sounds like just adding on_added for now is the consensus. I'll update the issue.

Edit: probably makes sense to create a new issue. I want it to be a "good first issue" kinda issue, so someone new could implement it if they so chose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-Easy just needs to be implemented enhancement adds or requests a new feature question causes uncertainty widget concerns a particular widget
Projects
None yet
Development

No branches or pull requests

3 participants