UI- Interaction Released state for nodes

[Leinnan]

Objective

Fixes #5769

Solution

Instead of changing node state from Pressed to None on click release, it first checks if pointer is still above element. If not, just go to None state. If yes changes it to Released state allowing for more familiar user experience (users expect action to happen on release, not click, also if we start pressing on button but release it with cursor outside of it we don't want to make action happened).

---

Changelog

Added Released state to nodes Interaction enum

Migration Guide

• user would need to cover one more case, so I guess it is a breaking change (?)

// 0.10
fn button_system(
    mut interaction_query: Query<
        (&Interaction, &mut BackgroundColor, &Children),
        (Changed<Interaction>, With<Button>),
    >,
    mut text_query: Query<&mut Text>,
) {
    for (interaction, mut color, children) in &mut interaction_query {
        let mut text = text_query.get_mut(children[0]).unwrap();
        match *interaction {
            Interaction::Clicked => {
                text.sections[0].value = "Press".to_string();
                *color = PRESSED_BUTTON.into();
            }
            Interaction::Hovered => {
                text.sections[0].value = "Hover".to_string();
                *color = HOVERED_BUTTON.into();
            }
            Interaction::None => {
                text.sections[0].value = "Button".to_string();
                *color = NORMAL_BUTTON.into();
            }
        }
    }
}
// 0.11
fn button_system(
    mut interaction_query: Query<
        (&Interaction, &mut BackgroundColor, &Children),
        (Changed<Interaction>, With<Button>),
    >,
    mut text_query: Query<&mut Text>,
) {
    for (interaction, mut color, children) in &mut interaction_query {
        let mut text = text_query.get_mut(children[0]).unwrap();
        match *interaction {
            Interaction::Released => {
                text.sections[0].value = "Released".to_string();
                *color = PRESSED_BUTTON.into();
            }
            Interaction::Clicked => {
                text.sections[0].value = "Press".to_string();
                *color = PRESSED_BUTTON.into();
            }
            Interaction::Hovered => {
                text.sections[0].value = "Hover".to_string();
                *color = HOVERED_BUTTON.into();
            }
            Interaction::None => {
                text.sections[0].value = "Button".to_string();
                *color = NORMAL_BUTTON.into();
            }
        }
    }
}

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[james7132]

This is a breaking change due to the addition of a new variant on a public not non_exhaustive enum. Can you fully flesh out the migration guide section?

[Leinnan]

Something like that in updated description would be enough? @james7132

[nicoburns]

This seems to be trying to squash a transient event into persistent state, and it seems like a bit of an impedance mismatch. Is there any reason why UI click events (that target a specific UI node) can't go through the bevy events system like the global mouse events do?

[ickshonpe]

This seems to be trying to squash a transient event into persistent state, and it seems like a bit of an impedance mismatch. Is there any reason why UI click events (that target a specific UI node) can't go through the bevy events system like the global mouse events do?

Events aren't any better as you still need to query for the button if you are going to do anything with it:

fn button_system(
    mut interaction_event: EventReader<InteractionEvent>,
    mut button_query: Query<
        (&mut BackgroundColor, &Children),
        With<Button>,
    >,
    mut text_query: Query<&mut Text>,
) {
    for event in interaction_event.iter() {
        if let Ok((mut color, children)) = button_query.get_mut(event.target_id) {
            let mut text = text_query.get_mut(children[0]).unwrap();
            match event.interaction {
                Interaction::Released => {
                    text.sections[0].value = "Released".to_string();
                    *color = PRESSED_BUTTON.into();
                }
                Interaction::Clicked => {
                    text.sections[0].value = "Press".to_string();
                    *color = PRESSED_BUTTON.into();
                }
                Interaction::Hovered => {
                    text.sections[0].value = "Hover".to_string();
                    *color = HOVERED_BUTTON.into();
                }
                Interaction::None => {
                    text.sections[0].value = "Button".to_string();
                    *color = NORMAL_BUTTON.into();
                }
            }
        }
     }
}

[nicoburns]

That does seem better to me. Yes, you still have to query any components you want to update. But at least your callback will get called at exactly the right time. Otherwise you have to deal with things like transitions from Released to None.

[nicoburns]

Also, I wouldn't expect an enum Interaction event. I would expect separate events for Clicked, MouseDown, MouseUp, MouseOver, MouseOut. And no None event. To me the existing approach makes sense for tracking "current state" (i.e. mouse_is_pressed or element_is_hovered), but it doesn't make sense for tracking changes to that state: (i.e. mouse pressed down or mouse started hovering element).

[ickshonpe]

Yeah, I don't think we disagree really. The example was meant more to show how this pattern of creating a new system for every button to model basic responses like text changing on mouse over etc is terrible regardless of whether you use events or change detection or whatever.

[jakerr]

I've noticed quite a bit of strange behavior on main with the current button interaction example, and this PR has the same issues (I tried it locally, see the video). I feel while adding this new state it might be a good time to take a look and make sure it's behaving as you expect it to.

I put a few log lines in examples/ui/button.rs and while the Interaction variants do appear to be delivered correctly the button state gets out of sync with the state of the mouse.

Notice when I hold down a click, the log says "Clicked" but the button still says "Hover" until the click is released or the mouse moved.

If I just stay over the button without moving the mouse and click and hold, release, and then click and hold again, the button starts saying "Released" while held down, and "Press" while released, until the mouse is moved, then it finally says released again.

As I mentioned main has similar issues so I'd understand if this were deemed to be a separate issue.

button-states.mov

[Leinnan]

This seems to be trying to squash a transient event into persistent state, and it seems like a bit of an impedance mismatch. Is there any reason why UI click events (that target a specific UI node) can't go through the bevy events system like the global mouse events do?

I was thinking about that for a while (first part mostly) and decided to take a different approach that doesn't require change in base bevy code and came up with this: https://github.com/Leinnan/bevy_button_released_plugin
I think for my use it is good enough, and I will wait to see what is going to be introduced to handle that later on.

[Shatur]

Was surprised that Clicked happens on button press. Usually it happens on release and only if mouse cursor stays on the button.

[alice-i-cecile]

Tackling this space over in #15597. Thanks for providing this prior art; it was helpful when looking at designs <3