search

parttio / tinymce-for-flow

TinyMCE wrapper for Vaadin 10+
Other
8 stars 6 forks source link

updateValue() not called on content change when wrapped in aCustomField #30

Open SonReimer opened 5 months ago

SonReimer commented 5 months ago

When putting TinyMce in a CustomField to add some feature like label, tooltip etc., someone has to add a ValueChangeListener to TineMce calling CustomField.updateValue() to propagate values from TinyMce to CustomField. This is not intuitive and differs from the documentation of CustomField (see comments from Martin Vysny) I think this is because client side throws a tchange event instead of change !?! TinyMce 4.1.3/Vaadin 24.3.2

mstahv commented 5 months ago

Do you have a code snippet available that could be used to reproduce the issue?

SonReimer commented 5 months ago

Here it is: If you remove the ValueChangeListener in TineMceField.java:10, user changes in the editor will not be shown in Notification when closing. MainView.java.txt TinyMceField.java.txt

mstahv commented 5 months ago

Verified.

But I gotta say I'm quite surprised by the behaviour of CustomField. This described behaviour seems to depend on propagated change events withtin the custom field (that might contain a dozen of fields). I'd even go as far as claiming that it is a bug, as it might easily be there are obsolete value changes and unexpected side effects now happening. Consider some custom field having a filtering input -> there will be a ton of extra traffic and JVM execution. Might even explains some weird behaviour I recently had in one of my apps, but didn't debug further...

If/as the workaround is so simple, I'll leave this open for now before investigating further.

And cheers to @mvysny !

On a related topics. Whic are all field features you'd like? tooltip and label mentioned.

mstahv commented 5 months ago

Checked that at least the #28 is again broken if wrapped in CustomField (yet another webcomponent). I'd suspect that is somethign related to Dialog not to this component, similar hack to CustomField might do it 🤷‍♂️ Maybe it would be easier to implement those features directly to this component, not sure.

SonReimer commented 5 months ago

I think label and tooltip are sufficient. From my point of view placeholder and helper do not really make sense.

SonReimer commented 5 months ago

Using the mentioned workaround for updateValue() is sufficient for me.