Skip to content

Conversation

@h0lg
Copy link
Contributor

@h0lg h0lg commented May 13, 2024

Includes some clean up, refactoring and renaming in separate commits.

Please take a good hard look at my implementation of the AutoCompleteBox.ItemTemplate attribute. It works - but I'm not at all certain whether I've used the correct of the many (sadly sparsely documented and hard to understand) Attributes.define* variants.
Neither have I checked whether the attempted reference equality comparison works as intended with compiled template builders - because I'm not certain what the exact intent of the equality comparison is. My guess is to prevent unnecessary re-rendering? Either way, please have a closer look at that as well and test it - because I don't really know what to look out for beyond the working samples.

@h0lg h0lg changed the title Adding AutoCompleteBox builder overloads for setting ItemTemplate Adding AutoCompleteBox extension for setting ItemTemplate May 14, 2024
@edgarfgp edgarfgp requested a review from TimLariviere May 15, 2024 21:17
@h0lg
Copy link
Contributor Author

h0lg commented May 16, 2024

I've just added a few commits - unrelated to the ItemTemplate setting, but not to the AutoCompleteBox. They add an animation to the remote populated input example when a search is in progress.

Comment on lines 169 to 163
static member inline itemTemplate(this: WidgetBuilder<'msg, #IFabAutoCompleteBox>, template: 'item -> WidgetBuilder<'msg, 'widget>) =
this.AddScalar(AutoCompleteBox.ItemTemplate.WithValue(WidgetHelpers.compileTemplate template))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a good place to compile the builder (here named template) into a template? Or do I lose the ability to identify different builders for the equality comparison by doing so? I'm doing it here because I was unable to declare the ItemTemplate attribute value with the open generic signature of the builder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No here. This is already done on WidgetHelpers.buildItems as part of the WidgetBuilder constructor

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a second look. It is ok to have compileTemplate as a separate function.

  • template: 'item -> WidgetBuilder<'msg, 'widget> will be passing 'item and then will be unboxed when WidgetDataTemplate is building the template.

  • One thing we can do is to use #IFabControl instead of 'widget

Comment on lines 169 to 163
static member inline itemTemplate(this: WidgetBuilder<'msg, #IFabAutoCompleteBox>, template: 'item -> WidgetBuilder<'msg, 'widget>) =
this.AddScalar(AutoCompleteBox.ItemTemplate.WithValue(WidgetHelpers.compileTemplate template))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No here. This is already done on WidgetHelpers.buildItems as part of the WidgetBuilder constructor

@h0lg h0lg force-pushed the auto-complete/set-item-template branch from ebd9f20 to 9bfb5c6 Compare May 18, 2024 09:44
@edgarfgp edgarfgp removed the request for review from TimLariviere May 18, 2024 20:44
@h0lg h0lg force-pushed the auto-complete/set-item-template branch from 4fa1e6e to 37ac190 Compare May 20, 2024 13:07
@edgarfgp edgarfgp merged commit 9ffe3d0 into fabulous-dev:main May 20, 2024
@h0lg h0lg deleted the auto-complete/set-item-template branch May 20, 2024 21:24
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.

2 participants