Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Focusable #18

Closed
erikhofer opened this issue Aug 8, 2023 · 9 comments
Closed

Implement Focusable #18

erikhofer opened this issue Aug 8, 2023 · 9 comments

Comments

@erikhofer
Copy link

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
Copy link
Collaborator

mstahv commented Aug 8, 2023

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

@erikhofer
Copy link
Author

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
Copy link
Collaborator

mstahv commented Aug 8, 2023

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.

@mstahv
Copy link
Collaborator

mstahv commented Aug 8, 2023

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

@erikhofer
Copy link
Author

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
Copy link
Collaborator

mstahv commented Aug 15, 2023

The regression pushed me on framework development and I spent basically the rest of my week there 😬 Some very heavy stuff: vaadin/flow#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
Copy link
Collaborator

mstahv commented Sep 6, 2023

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

@mstahv mstahv closed this as completed Sep 6, 2023
@mstahv
Copy link
Collaborator

mstahv commented Sep 6, 2023

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
Copy link
Author

@mstahv Thank you! :)

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

No branches or pull requests

2 participants