vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

@Id should work in a nested template #2233

Open alvarezguille opened 7 years ago

alvarezguille commented 7 years ago

It seems that now it's not allowed on purpose according to #2149 but it really limits use and re-use of templates.

Current workaround is to instantiate and attach the nested template with code by replacing <child-template> tag with <slot></slot> and appending the child-template to parent-template in Java.

Legioth commented 7 years ago

In the general case, this is really an "impossible" feature to support since the element defined might be instantiated any number of times. One could in theory allow @Id Collection<ComponentType> instead of just @Id ComponentType for anything inside a sub template, but this would most likely not be very useful in practice.

The special case of <dom-if> is somewhat easer to deal with since the number of instances is always either 0 or 1, but the case when there are 0 instances would still be quite weird. Consider a template with <template is="dom-if" if="_condition(prop)"><input id="phantom"></template> and a corresponding @Id Input phantom field on the server. What should happen if doing phantom.getElement().callFunction("focus") while the condition is false?

  1. Runtime exception because phantom is currently null.
  2. Runtime exception because the element is currently in a phantom state.
  3. Enqueue the action so that the element would become focused later on when the condition eventually changes.
  4. Nothing.
  5. Something else, what?

Another similar question is what phantom.getElement().getParent() should return and whether the component's onAttach handler would be run.

Polymer also takes a similar stance by not making elements inside subtemplates available as this.$.<elementId>.

It is still a very relevant use case to conditionally leave out some parts of the element hierarchy defined by the template. I'm wondering if it would be more straightforward and logical if the way you do that with instances managed from the server would be through that instance instead of doing it through the template, i.e. phantom.setVisible(false) as suggested in https://github.com/vaadin/flow/issues/2187.

alvarezguille commented 7 years ago

There was a confusion about template in template and nested template, this issue remains open as a feature request for nested template support, and #2245 was created as a bug for @Id support in template in template.

vlukashov commented 6 years ago

The lack of this feature does affect the developer experience. And—from my perspective—in case of conditional templates it makes the DX a lot worse.

For example:

@Id("storefront-item")
private StorefrontItem storefrontItem;

@Id("order-detail")
private OrderDetail orderDetail;

has to be turned into

private StorefrontItem storefrontItem;
private OrderDetail orderDetail;

public StorefrontItemDetailWrapper() {
    storefrontItem = new StorefrontItem();
    storefrontItem.getElement().setAttribute("slot", "storefront-item");
    getElement().appendChild(storefrontItem.getElement());

    orderDetail = new OrderDetail();
    orderDetail.getElement().setAttribute("slot", "order-detail");
    getElement().appendChild(orderDetail.getElement());
}

Suggestion: make the elements inside the conditional templates Optional. For example:

@Id("storefront-item")
private Optional<StorefrontItem> storefrontItem;

@Id("order-detail")
private Optional<OrderDetail> orderDetail;
Legioth commented 6 years ago

The problem is that we'd have to build custom support for each potential use of templates.

Some examples:

<template is="dom-if" condition="[[model.value]]">
  <vaadin-button id="button1">Button 1</vaadin-button>
</template>

<template is="dom-if" condition="[[model.value]]" restamp>
  <vaadin-button id="button2>Button 2</vaadin-button>
</template>

<template is="dom-if" condition="_computedCondition([[model.value]])">
  <vaadin-button id="button3">Button 3</vaadin-button>
</template>

<template is="dom-repeat" items="[[model.items]]">
  <vaadin-button id="button4">Button [[item.value]]</vaadin-button>
</template>

In each case, there's a different logic for what value the @Id Java field should have.

In addition to these, there are also known templates for at least <vaadin-grid>, <vaadin-combo-box>, <vaadin-dialog> and <vaadin-context-menu>, not to mention all the third party components that might also support templates in different ways. Each of them have different logic for when the template is stamped and how many copies there are.

As long as dom-if is mainly used for presentational reasons, you can bind to the hidden attribute instead.

We could start out by implementing support for only simple dom-if without restamp and no support for computed properties, but I fear it would open up an endless can of worms.

denis-anisimov commented 6 years ago

This should be fixed by #3057.

denis-anisimov commented 6 years ago

Closing as a duplicate of #3057.

Legioth commented 6 years ago

I don't understand how this would be fixed by #3057. Is it now possible to use @Id for an element inside <template is="dom-repeat">?

denis-anisimov commented 6 years ago

Closed by mistake.