parttio / tinymce-for-flow

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

Implement Focusable #18

Closed erikhofer closed 10 months ago

erikhofer commented 11 months ago

The component already has a focus() method. It would be really useful if it implemented com.vaadin.flow.component.Focusable<T>. I think in this case, support for blurring needs to be added.

mstahv commented 11 months ago

Do you happen to need the blur() method? Didn't quickly find a way to implement that in TinyMC JS widget.

erikhofer commented 11 months ago

Oh, interesting that blur is not exposed just like focus. I think for us this would be okay. What would be helpful, however, is implementing BlurNotifier.addBlurListener() that comes with Focusable. This would need to be wired to the blur event which looks possible: https://www.tiny.cloud/docs/tinymce/6/events/#supported-browser-native-events

mstahv commented 11 months ago

Pushed a branch where Focusable is implemented. Known issues:

mstahv commented 11 months ago

Also there seems to be a bug(or regression) with Chrome, Safari works fine 😬

erikhofer commented 11 months ago

Pushed a branch where Focusable is implemented. Known issues:

  • blur() not supported (as discussed), throws UOE

  • blur listener is fired also when opening menus in the editor

I wonder how the latter should work and if it can be changed easily. If your think this would be helpful, then I'm fine committing that to the main branch.

Thanks for the effort :) Looks good to me so far. I guess the behaviour is correct because when you open a menu, the focus of the text area (as indicated by the blinking cursor) is actually lost.

mstahv commented 11 months ago

The regression pushed me on framework development and I spent basically the rest of my week there 😬 Some very heavy stuff: https://github.com/vaadin/flow/pull/17410

I hope I can get that PR to framework soon and then push out a new version with this and vastly improved content synchronisation.

mstahv commented 10 months ago

Fixed in 4.0.1 (currently syncing to maven central). Needs Vaadin 24.1.8 or newer!

mstahv commented 10 months ago

Better make that 4.0.2, 4.0.1 had a regression that it didn't fire value change when formatting was changed via buttons/menus.

erikhofer commented 10 months ago

@mstahv Thank you! :)