-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Allowed for linking FocusTracker instances together and propagate the focus state #17206
Conversation
…despite a pending blur timeout.
…nt and handle its focus interactions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to admit that full and deep code review would take a lot of my time, trying to get to the roots of how exactly this work.
I added some comments to places which weren't immediately obvious to me, but after going through it for the second time, I think I understood why they are like that. It seems to me that introduced changes make sense.
But I cannot tell whether something is missing, or some other place should be modified too.
I know you put a lot of (hehe) focus into this issue and this is a third (?) try to get it working in a way that won't introduce breaking changes. So I think I have to lean on your experience in this matter.
Happy to talk about the bits which I commented on, I wonder if I got it right.
packages/ckeditor5-ui/src/dropdown/menu/dropdownmenurootlistview.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please finish the todos and the one thing regarding the private member, and it should be fine.
Suggested merge commit message (convention)
Fix (ui): The dropdown menu component should not cause an editor blur if used in a
BalloonToolbar
while the user hovers a nested menu. Closes #17277.Feature (utils): Made
FocusTracker
extendable with otherFocusTracker
instances to allow logical focus tracking across separate DOM sub-trees.Additional information
Closes https://github.com/cksource/ckeditor5-commercial/issues/6633.
Comes with https://github.com/cksource/ckeditor5-commercial/pull/6672
TODOs:
FT#_label
debug lines from features (after QA)