Trait HasId: remove or expand usage?

[dhardy]

Some methods like fn EventState::redraw take id: impl HasId, allowing calling like:

    cx.redraw(self);

(instead of cx.redraw(self.id())).

Complication: sometimes one needs to use cx.redraw(&self) to avoid "moving" self.

#540 expanded usage to include fn depress_with_key since this has significant usage. Quite a few other functions could support this trait, for uniformity's sake if nothing else.

Or the trait could be removed altogether: this is a minor simplification requiring a minor amount more typing when calling methods like fn EventState::redraw.

Source notes

diff --git a/crates/kas-core/src/event/cx/key.rs b/crates/kas-core/src/event/cx/key.rs
index d45512147a..e80d5c2f93 100644
--- a/crates/kas-core/src/event/cx/key.rs
+++ b/crates/kas-core/src/event/cx/key.rs
@@ -115,6 +115,7 @@
     /// Key focus implies sel focus (see [`Self::request_sel_focus`]) and
     /// navigation focus.
     #[inline]
+    // TODO: use HasId? No: little usage and would cause borrow issues
     pub fn request_key_focus(&mut self, target: Id, ime: Option<ImePurpose>, source: FocusSource) {
         if self.nav_focus.as_ref() != Some(&target) {
             self.set_nav_focus(target.clone(), source);
@@ -142,6 +143,7 @@
     ///
     /// When key focus is lost, [`Event::LostSelFocus`] is sent.
     #[inline]
+    // TODO: use HasId? Little usage
     pub fn request_sel_focus(&mut self, target: Id, source: FocusSource) {
         if self.nav_focus.as_ref() != Some(&target) {
             self.set_nav_focus(target.clone(), source);
@@ -173,6 +175,7 @@
     /// may be changed by modifier keys.
     ///
     /// Does nothing when `code` is `None`.
+    // TODO: use HasId? Maybe: significant usage, at least half of which doesn't have borrow issues
     pub fn depress_with_key(&mut self, id: Id, code: impl Into<Option<PhysicalKey>>) {
         fn inner(state: &mut EventState, id: Id, code: PhysicalKey) {
             if state
@@ -215,6 +218,7 @@
     ///
     /// If `alt_bypass` is true, then this layer's access keys will be
     /// active even without Alt pressed (but only highlighted with Alt pressed).
+    // TODO: use HasId? Little usage
     pub fn new_access_layer(&mut self, id: Id, alt_bypass: bool) {
         self.access_layers.insert(id, (alt_bypass, HashMap::new()));
     }
@@ -268,6 +272,7 @@
 
     /// End Input Method Editor focus on `target`, if present
     #[inline]
+    // TODO: use HasId? Little usage
     pub fn cancel_ime_focus(&mut self, target: Id) {
         if let Some(pending) = self.pending_sel_focus.as_mut() {
             if pending.target.as_ref() == Some(&target) {
diff --git a/crates/kas-core/src/event/cx/mod.rs b/crates/kas-core/src/event/cx/mod.rs
index f56e4b92b6..714d674a1f 100644
--- a/crates/kas-core/src/event/cx/mod.rs
+++ b/crates/kas-core/src/event/cx/mod.rs
@@ -269,6 +269,7 @@
     ///
     /// Disabling a widget clears navigation, selection and key focus when the
     /// target is disabled, and also cancels press/pan grabs.
+    // TODO: use HasId? Little point
     pub fn set_disabled(&mut self, target: Id, disable: bool) {
         if disable {
             self.cancel_event_focus(&target);
@@ -293,6 +294,7 @@
     ///
     /// This is equivalent to calling [`Self::action`] with [`Action::REDRAW`].
     #[inline]
+    // TODO: keep? Significant usage, though approx half use `&self` to avoid borrow issue.
     pub fn redraw(&mut self, id: impl HasId) {
         self.action(id, Action::REDRAW);
     }
@@ -301,6 +303,7 @@
     ///
     /// This is equivalent to calling [`Self::action`] with [`Action::RESIZE`].
     #[inline]
+    // TODO: keep? 33 uses of HasId.
     pub fn resize(&mut self, id: impl HasId) {
         self.action(id, Action::RESIZE);
     }
@@ -326,6 +329,7 @@
     /// required. Should a widget's size requirements change, these will only
     /// affect the UI after a reconfigure action.
     #[inline]
+    // TODO: keep? 27 possible uses of HasId.
     pub fn action(&mut self, id: impl HasId, action: Action) {
         fn inner(cx: &mut EventState, id: Id, mut action: Action) {
             if action.contains(Action::UPDATE) {
@@ -364,6 +368,7 @@
     /// Request update to widget `id`
     ///
     /// This will call [`Events::update`] on `id`.
+    // TODO: use HasId? Little usage
     pub fn request_update(&mut self, id: Id) {
         self.pending_update = if let Some(id2) = self.pending_update.take() {
             Some(id.common_ancestor(&id2))
@@ -403,8 +408,6 @@
 impl<'a> EventCx<'a> {
     /// Configure a widget
     ///
-    /// Note that, when handling events, this method returns the *old* state.
-    ///
     /// This is a shortcut to [`ConfigCx::configure`].
     #[inline]
     pub fn configure(&mut self, mut widget: Node<'_>, id: Id) {
diff --git a/crates/kas-core/src/event/cx/nav.rs b/crates/kas-core/src/event/cx/nav.rs
index e5f3b52a64..2f8101a601 100644
--- a/crates/kas-core/src/event/cx/nav.rs
+++ b/crates/kas-core/src/event/cx/nav.rs
@@ -102,6 +102,7 @@
     /// Normally, [`Tile::navigable`] will return true for widget `id` but this
     /// is not checked or required. For example, a `ScrollLabel` can receive
     /// focus on text selection with the mouse.
+    // TODO: use HasId? No.
     pub fn set_nav_focus(&mut self, id: Id, source: FocusSource) {
         self.pending_nav_focus = PendingNavFocus::Set {
             target: Some(id),
@@ -140,6 +141,7 @@
     /// Only one widget can be a fallback, and the *first* to set itself wins.
     /// This is primarily used to allow scroll-region widgets to
     /// respond to navigation keys when no widget has focus.
+    // TODO: use HasId? 5 possible uses.
     pub fn register_nav_fallback(&mut self, id: Id) {
         if self.nav_fallback.is_none() {
             log::debug!(target: "kas_core::event","register_nav_fallback: id={id}");
diff --git a/crates/kas-core/src/event/cx/press.rs b/crates/kas-core/src/event/cx/press.rs
index fe30dd9d2b..fbb7c8cdb6 100644
--- a/crates/kas-core/src/event/cx/press.rs
+++ b/crates/kas-core/src/event/cx/press.rs
@@ -224,6 +224,7 @@
     /// is returned. It is expected that the requested press/pan events are all
     /// "used" ([`Used`]).
     #[inline]
+    // TODO: use HasId? Little usage
     pub fn grab(&self, id: Id, mode: GrabMode) -> GrabBuilder {
         GrabBuilder {
             id,
@@ -236,12 +237,14 @@
 
     /// Convenience wrapper for [`Self::grab`] using [`GrabMode::Click`]
     #[inline]
+    // TODO: use HasId? 3 possible uses
     pub fn grab_click(&self, id: Id) -> GrabBuilder {
         self.grab(id, GrabMode::Click)
     }
 
     /// Convenience wrapper for [`Self::grab`] using [`GrabMode::Grab`]
     #[inline]
+    // TODO: use HasId? 4 possible uses
     pub fn grab_move(&self, id: Id) -> GrabBuilder {
         self.grab(id, GrabMode::Grab)
     }
diff --git a/crates/kas-core/src/event/cx/send.rs b/crates/kas-core/src/event/cx/send.rs
index 89b1909915..991c8fd486 100644
--- a/crates/kas-core/src/event/cx/send.rs
+++ b/crates/kas-core/src/event/cx/send.rs
@@ -38,6 +38,7 @@
     /// The message is pushed to the message stack. The target widget may
     /// [pop](EventCx::try_pop) or [peek](EventCx::try_peek) the message from
     /// [`Events::handle_messages`].
+    // TODO: use HasId? No
     pub fn send<M: Debug + 'static>(&mut self, id: Id, msg: M) {
         self.send_erased(id, Erased::new(msg));
     }
@@ -45,6 +46,7 @@
     /// Push a type-erased message to the stack
     ///
     /// This is a lower-level variant of [`Self::send`].
+    // TODO: use HasId? no
     pub fn send_erased(&mut self, id: Id, msg: Erased) {
         self.send_queue.push_back((id, msg));
     }
@@ -57,6 +59,7 @@
     ///
     /// The future must resolve to a message on completion. Its message is then
     /// sent to `id` via stack traversal identically to [`Self::send`].
+    // TODO: use HasId? little usage
     pub fn send_async<Fut, M>(&mut self, id: Id, fut: Fut)
     where
         Fut: IntoFuture<Output = M> + 'static,
@@ -68,6 +71,7 @@
     /// Send a type-erased message to `id` via a [`Future`]
     ///
     /// This is a low-level variant of [`Self::send_async`].
+    // TODO: use HasId? little usage
     pub fn send_async_erased<Fut>(&mut self, id: Id, fut: Fut)
     where
         Fut: IntoFuture<Output = Erased> + 'static,
@@ -86,6 +90,7 @@
     /// and [`Self::send_async`]; if a different multi-threaded executor is
     /// available, that may be used instead. See also [`async_global_executor`]
     /// documentation of configuration.
+    // TODO: use HasId? Maybe (common to send to self)
     #[cfg(feature = "spawn")]
     pub fn send_spawn<Fut, M>(&mut self, id: Id, fut: Fut)
     where
diff --git a/crates/kas-core/src/event/cx/timer.rs b/crates/kas-core/src/event/cx/timer.rs
index 1254a20886..3950a71d09 100644
--- a/crates/kas-core/src/event/cx/timer.rs
+++ b/crates/kas-core/src/event/cx/timer.rs
@@ -61,6 +61,7 @@
     ///
     /// Multiple timer requests with the same `id` and `handle` are merged
     /// (see [`TimerHandle`] documentation).
+    // TODO: use HasId? Maybe
     pub fn request_timer(&mut self, id: Id, handle: TimerHandle, delay: Duration) {
         let time = Instant::now() + delay;
         if let Some(row) = self
@@ -96,6 +97,7 @@
     /// [`Draw::animate`](crate::draw::Draw::animate) instead.
     ///
     /// It is expected that `handle.earliest() == true` (style guide).
+    // TODO: use HasId? Maybe
     pub fn request_frame_timer(&mut self, id: Id, handle: TimerHandle) {
         debug_assert!(handle.earliest());
         self.frame_updates.insert((id, handle));
diff --git a/crates/kas-core/src/event/event.rs b/crates/kas-core/src/event/event.rs
index ddd772572b..8869dbde12 100644
--- a/crates/kas-core/src/event/event.rs
+++ b/crates/kas-core/src/event/event.rs
@@ -239,6 +239,7 @@
     /// -   `Event::Command(cmd, _)` where [`cmd.is_activate()`](Command::is_activate)
     ///
     /// The method calls [`EventState::depress_with_key`] on activation.
+    // TODO: use HasId? Maybe, but potentially confusing and may cause borrow issues
     pub fn on_activate<F: FnOnce(&mut EventCx) -> IsUsed>(
         self,
         cx: &mut EventCx,
diff --git a/crates/kas-core/src/theme/draw.rs b/crates/kas-core/src/theme/draw.rs
index 804bfc706f..d187c83df5 100644
--- a/crates/kas-core/src/theme/draw.rs
+++ b/crates/kas-core/src/theme/draw.rs
@@ -107,6 +107,7 @@
     /// of widget state (e.g. is disabled, is under the mouse, has key focus).
     /// Usually you don't need to worry about this since the `#[widget]` macro
     /// injects a call to this method at the start of [`Layout::draw`].
+    // TODO: use HasId? Maybe.
     pub fn set_id(&mut self, id: Id) {
         self.id = id;
     }
diff --git a/crates/kas-core/src/window/popup.rs b/crates/kas-core/src/window/popup.rs
index 0db332b107..be57e4fa56 100644
--- a/crates/kas-core/src/window/popup.rs
+++ b/crates/kas-core/src/window/popup.rs
@@ -165,6 +165,7 @@
             &mut self,
             cx: &mut EventCx,
             data: &W::Data,
+    // TODO: use HasId? Maybe, but confusing?
             parent: Id,
             set_focus: bool,
         ) -> bool {
diff --git a/crates/kas-widgets/src/adapt/adapt_cx.rs b/crates/kas-widgets/src/adapt/adapt_cx.rs
index b2aaac564f..72b9eab025 100644
--- a/crates/kas-widgets/src/adapt/adapt_cx.rs
+++ b/crates/kas-widgets/src/adapt/adapt_cx.rs
@@ -22,6 +22,7 @@
 impl<'a: 'b, 'b> AdaptEventCx<'a, 'b> {
     /// Construct
     #[inline]
+    // TODO: use HasId? No.
     pub fn new(cx: &'b mut EventCx<'a>, id: Id) -> Self {
         AdaptEventCx { cx, id }
     }
@@ -87,6 +88,7 @@
 impl<'a: 'b, 'b> AdaptConfigCx<'a, 'b> {
     /// Construct
     #[inline]
+    // TODO: use HasId? No.
     pub fn new(cx: &'b mut ConfigCx<'a>, id: Id) -> Self {
         AdaptConfigCx { cx, id }
     }