Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Calling setText for Button appends the text to an existing caption #43

Closed
johannesh2 opened this issue Feb 27, 2018 · 9 comments
Closed

Comments

@johannesh2
Copy link

Having a template with <vaadin-button>Click</vaadin-button>
And calling later in Java
myButton.setText("Me!")

Expected:
Button has text "Me!"
Actual:
Button has text "Click Me!"

@pleku
Copy link
Contributor

pleku commented Feb 27, 2018

We need to find a way to clear that text, as the method is suppose to set the text, not append it.

@Legioth
Copy link
Member

Legioth commented Feb 27, 2018

The basic issue is that setText is implemented to first remove all children, and then add a single text node. The problem is that removing all children is a no-op in this case when the server-side API isn't aware of the content that is there only through the template. There are thus different obvious ways of approaching this:

  1. Make removeAllChildren() actually remove all children, not only those that the server knows about
  2. Make the element instance aware of content that is supposedly there because of the template

@denis-anisimov
Copy link

denis-anisimov commented Feb 27, 2018

The fix of this simple issue which is definitely confusing may introduce terrible consequences.

At the moment it's just a confusion.
But after fixing this it may become a catastrophe.

Let's imagine I have :

<template>
<div id="foo>
  Text
   <div id="bar">Bar<div>
</div>
</template>

And I have both divs injected via @Id :

@Id("foo")
private Div child1;
@Id("bar")
private Div child2;

Now we should remove everything inside the first div along with nested div.
As a result the child2 refers to non-existing element and there will be huge confusion why it doesn't work.

Also the issue with setText is just one aspect of non-working Element API.
The element injected via @Id also doesn't contain children though it may contain children on the client side. And it's the same situation. The theoretical question is : why children methods doesn't work as expected ?
Again, if we allow to remove children from the injected element then what happens with those children which has been also injected ?

I would say the main confusion here is @Id concept as a whole.
It has not been thought through enough.
I would say that Element API (along with Component ) should not be used for injected elements at all. That't the first impression. But in this case it becomes almost useless.

Anyway. What I'm definitely sure: let's don't fix it SOMEHOW. We may introduce much more serious issues than just a confusion as it's for now.

We need to go through all usecases and find solution for each of them.
And all of them should be taken into account carefully with a proper design.

This issue requires a serious discussion instead of quick fix proposition.

@Legioth
Copy link
Member

Legioth commented Feb 27, 2018

child2 would then refer to an element that isn't attached on the client, just as it would if the corresponding DOM structure would be built using only the Element API without any templates. This is what the application developer expects, regardless of whether they're using @Id or not.

There might be some small differences such as the fact that child2 is still attached (as a virtual child) on the server, but that isn't any worse than the situation with ComponentRenderer in Grid where we also can have elements that are attached according to the server even though they are not attached to the actual DOM in the browser.

@mvysny
Copy link
Member

mvysny commented Mar 6, 2018

@denis-anisimov I agree. The source of the problem as I see it is that the server-side Button (or any Vaadin10 Component for that matter) will not receive any DOM content when created from a Polymer Template. This will cause any API that depend on the DOM content to fail and/or return incorrect results:

  • Button.getText() returns empty String even though the button has text client-side;
  • You can't create a dialog with a "Create/Save" button, altering the caption depending on whether the entity already exists in the database or not - the button will append the caption instead of setting it.
  • Layouts will report no children probably
  • And others.

I believe this to be a major inconsistency. Not only this kills browserless testing for Polymer Templates, it also makes the server abstraction leak in various ways. The user will have to learn that Buttons and components nested inside of the Polymer Template are inferior than server-side constructed Buttons and behave differently.

@pleku
Copy link
Contributor

pleku commented Mar 14, 2018

This issue was also reported in vaadin/flow#3699.

@pleku pleku added this to the Before 1.0 RC milestone Mar 14, 2018
@pleku
Copy link
Contributor

pleku commented Mar 14, 2018

Based on the amount of feedback on this and on what happens when components added to a Id-mapped layout go to unexpected location, we need to resolve this before RC and release a Beta version with the fix.

@Legioth
Copy link
Member

Legioth commented Mar 15, 2018

Will be fixed through vaadin/flow#3713

@denis-anisimov
Copy link

As I understand this will be fixed automatically once removeAllChildren(); is implemented in the new way.

So may this is only about IT test.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants