vaadin / hilla

Build better business applications, faster. No more juggling REST endpoints or deciphering GraphQL queries. Hilla seamlessly connects Spring Boot and React to accelerate application development.
https://hilla.dev
Apache License 2.0
899 stars 56 forks source link

Fusion field binding does not work with async items #270

Open marcushellberg opened 3 years ago

marcushellberg commented 3 years ago

Version: Vaadin Fusion 18.

Issue: When using field on a combo-box, setting the items asynchronously does not work as expected.

In the following example, the first item is selected no matter what the model value is. On subsequent renders, the items are present when creating the field, and it works as expected.

@customElement("item-form")
export class ItemForm extends MobxLitElement {
  @internalProperty()
  private categories: Category[] = [];
  private binder = new Binder(this, ListItemModel);

  render() {
    const { model } = this.binder;
    return html`
      <vaadin-combo-box
        label="Category"
        ...=${field(model.category)}
        .items=${store.categories}
        item-label-path="name"
      ></vaadin-combo-box>
    `;
  }

  async connectedCallback() {
    super.connectedCallback();
    this.categories = await getCategories();
  }
}

Here is a project where you can reproduce the issue. Notice that on the list view, the combo-box shows the first item instead of the correct item on the first load (when the items are set async). If you navigate to the other view and back, it works as intended (because the items are already in the store when the field is created).

Legioth commented 3 years ago

How should this even work from the user's point of view? What should be shown in the text field of the combo box when items is empty? It might not be the best user experience if it would flicker to first show up as blank and then the actual label would appear moments later.

Should there be a distinction between items=undefined and items=[]?

What about just adding if (!store.categories) return "Loading..." to the beginning of render()? Maybe there should be a mechanism in Fusion to make it really easy to just show a placeholder until all "required" data dependencies are loaded, similar to <Suspense> in React?

marcushellberg commented 3 years ago

Using a render guard is one option. But it may not be obvious to the user. In this case, the first item gets selected visually, but not bound to the value object, which is confusing.

I'm not sure what the right approach would be. Maybe disable the field while items=undefined?