Disable UI node Interaction when ComputedVisibility is false (bev…

…yengine#5361)

# Objective

UI nodes can be hidden by setting their `Visibility` property. Since bevyengine#5310 was merged, this is now ergonomic to use, as visibility is now inherited.

However, UI nodes still receive (and store) interactions when hidden, resulting in surprising hidden state (and an inability to otherwise disable UI nodes.

## Solution

Fixes bevyengine#5360.

I've updated the `ui_focus_system` to accomplish this in a minimally intrusive way, and updated the docs to match.

**NOTE:** I have not added automated tests to verify this behavior, as we do not currently have a good testing paradigm for `bevy_ui`. I'm not thrilled with that by any means, but I'm not sure fixing it is within scope.

## Paths not taken

### Separate `Disabled` component

This is a much larger and more controversial change, and not well-scoped to UI.
Furthermore, it is extremely rare that you want hidden UI elements to function: the most common cases are for things like changing tabs, collapsing elements or so on.
Splitting this behavior would be more complex, and substantially violate user expectations.

### A separate limbo world

Mentioned in the linked issue. Super cool, but all of the problems  of the `Disabled` component solution with a whole new RFC-worth of complexity.

### Using change detection to reduce the amount of redundant work

Adds a lot of complexity for questionable performance gains. Likely involves a complete refactor of the entire system.

We simply don't have the tests or benchmarks here to justify this.

## Changelog

- UI nodes are now always in an `Interaction::None` state while they are hidden (via the `ComputedVisibility` component).

crates/bevy_ui/src/focus.rs

@@ -9,6 +9,7 @@ use bevy_input::{mouse::MouseButton, touch::Touches, Input};
 use bevy_math::Vec2;
 use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize};
 use bevy_render::camera::{Camera, RenderTarget};
+use bevy_render::view::ComputedVisibility;
 use bevy_transform::components::GlobalTransform;
 use bevy_utils::FloatOrd;
 use bevy_window::Windows;
@@ -18,6 +19,17 @@ use smallvec::SmallVec;
 /// Describes what type of input interaction has occurred for a UI node.
 ///
 /// This is commonly queried with a `Changed<Interaction>` filter.
+///
+/// Updated in [`ui_focus_system`].
+///
+/// If a UI node has both [`Interaction`] and [`ComputedVisibility`] components,
+/// [`Interaction`] will always be [`Interaction::None`]
+/// when [`ComputedVisibility::is_visible()`] is false.
+/// This ensures that hidden UI nodes are not interactable,
+/// and do not end up stuck in an active state if hidden at the wrong time.
+///
+/// Note that you can also control the visibility of a node using the [`Display`](crate::ui_node::Display) property,
+/// which fully collapses it during layout calculations.
 #[derive(
     Component, Copy, Clone, Default, Eq, PartialEq, Debug, Reflect, Serialize, Deserialize,
 )]
@@ -51,6 +63,8 @@ pub struct State {
 }
 
 /// The system that sets Interaction for all UI elements based on the mouse cursor activity
+///
+/// Entities with a hidden [`ComputedVisibility`] are always treated as released.
 pub fn ui_focus_system(
     mut state: Local<State>,
     camera: Query<(&Camera, Option<&UiCameraConfig>)>,
@@ -64,6 +78,7 @@ pub fn ui_focus_system(
         Option<&mut Interaction>,
         Option<&FocusPolicy>,
         Option<&CalculatedClip>,
+        Option<&ComputedVisibility>,
     )>,
 ) {
     // reset entities that were both clicked and released in the last frame
@@ -76,7 +91,7 @@ pub fn ui_focus_system(
     let mouse_released =
         mouse_button_input.just_released(MouseButton::Left) || touches_input.any_just_released();
     if mouse_released {
-        for (_entity, _node, _global_transform, interaction, _focus_policy, _clip) in
+        for (_entity, _node, _global_transform, interaction, _focus_policy, _clip, _visibility) in
             node_query.iter_mut()
         {
             if let Some(mut interaction) = interaction {
@@ -111,7 +126,22 @@ pub fn ui_focus_system(
     let mut moused_over_z_sorted_nodes = node_query
         .iter_mut()
         .filter_map(
-            |(entity, node, global_transform, interaction, focus_policy, clip)| {
+            |(entity, node, global_transform, interaction, focus_policy, clip, visibility)| {
+                // Nodes that are not rendered should not be interactable
+                if let Some(computed_visibility) = visibility {
+                    if !computed_visibility.is_visible() {
+                        // Reset their interaction to None to avoid strange stuck state
+                        if let Some(mut interaction) = interaction {
+                            // We cannot simply set the interaction to None, as that will trigger change detection repeatedly
+                            if *interaction != Interaction::None {
+                                *interaction = Interaction::None;
+                            }
+                        }
+
+                        return None;
+                    }
+                }
+
                 let position = global_transform.translation();
                 let ui_position = position.truncate();
                 let extents = node.size / 2.0;

crates/bevy_ui/src/ui_node.rs

@@ -66,6 +66,8 @@ impl AddAssign<f32> for Val {
 #[reflect(Component, Default, PartialEq)]
 pub struct Style {
     /// Whether to arrange this node and its children with flexbox layout
+    ///
+    /// If this is set to [`Display::None`], this node will be collapsed.
     pub display: Display,
     /// Whether to arrange this node relative to other nodes, or positioned absolutely
     pub position_type: PositionType,
@@ -212,14 +214,19 @@ pub enum Direction {
     RightToLeft,
 }
 
-/// Whether to use Flexbox layout
+/// Whether to use a Flexbox layout model.
+///
+/// Part of the [`Style`] component.
 #[derive(Copy, Clone, PartialEq, Eq, Debug, Default, Serialize, Deserialize, Reflect)]
 #[reflect_value(PartialEq, Serialize, Deserialize)]
 pub enum Display {
-    /// Use flexbox
+    /// Use Flexbox layout model to determine the position of this [`Node`].
     #[default]
     Flex,
-    /// Use no layout, don't render this node and its children
+    /// Use no layout, don't render this node and its children.
+    ///
+    /// If you want to hide a node and its children,
+    /// but keep its layout in place, set its [`Visibility`](bevy_render::view::Visibility) component instead.
     None,
 }