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

Remove input data from widget layout syntax items #478

Merged
merged 6 commits into from
Feb 17, 2025
Merged

Remove input data from widget layout syntax items #478

merged 6 commits into from
Feb 17, 2025

Conversation

dhardy
Copy link
Collaborator

@dhardy dhardy commented Feb 17, 2025

Previously, widgets embedded in input data could support input data. This was rarely ever used within custom widgets but commonly used as part of "widget constructor macros" like column!, until #476 made those return named widgets (like Column).

We can now remove this feature. Motivation (aside from some simpler code) is that this avoids some confusing errors. For example, the last commit here simply in-lines a widget field which is never accessed nor uses input data:

diff --git a/crates/kas-widgets/src/combobox.rs b/crates/kas-widgets/src/combobox.rs
index b7a2f49d..c8074527 100644
--- a/crates/kas-widgets/src/combobox.rs
+++ b/crates/kas-widgets/src/combobox.rs
@@ -29,7 +29,7 @@ impl_scope! {
     /// when selected. If a handler is specified via [`Self::with`] or
     /// [`Self::with_msg`] then this message is passed to the handler and not emitted.
     #[widget {
-        layout = frame!(row! [self.label, self.mark])
+        layout = frame!(row! [self.label, Mark::new(MarkStyle::Point(Direction::Down))])
             .with_style(FrameStyle::Button)
             .align(AlignHints::CENTER);
         navigable = true;
@@ -40,8 +40,6 @@ impl_scope! {
         #[widget(&())]
         label: Label<String>,
         #[widget(&())]
-        mark: Mark,
-        #[widget(&())]
         popup: Popup<AdaptEvents<Column<Vec<MenuEntry<V>>>>>,
         active: usize,
         opening: bool,
@@ -233,7 +231,6 @@ impl<A, V: Clone + Debug + Eq + 'static> ComboBox<A, V> {
         ComboBox {
             core: Default::default(),
             label,
-            mark: Mark::new(MarkStyle::Point(Direction::Down)),
             popup: Popup::new(
                 AdaptEvents::new(Column::new(entries)).on_messages(|cx, _, _| {
                     if let Some(_) = cx.try_peek::<V>() {

Without the removal of this feature, that diff results in the following error:

error[E0412]: cannot find type `A` in this scope
  --> crates/kas-widgets/src/combobox.rs:62:21
   |
62 |         type Data = A;
   |                     ^ not found in this scope

For more information about this error, try `rustc --explain E0412`.
error: could not compile `kas-widgets` (lib) due to 1 previous error

Why? Because widgets declared in-line in layout syntax for custom widgets are placed in the widget_core!() struct and expected to support the custom widget's data type; in this case, the field is expected to implement Widget<Data = A> — but here A is a generic of the (outer) widget type, not the core. We could solve that error by copying all generics from the outer widget to the core (or attempt to infer which generics are actually required), but this solution is cleaner. (It's also cleaner because we didn't want to use this input data anyway, and usually don't for widgets declared in layout syntax because these are usually static.)

@dhardy dhardy merged commit 21c94e7 into master Feb 17, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant