-
Notifications
You must be signed in to change notification settings - Fork 6.8k
perf(focus-monitor): optimize event registration #18667
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
Conversation
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.
LGTM. @mmalerba should have a look too.
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.
LGTM
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.
LGTM
Improve focus-monitor scalability by implementing event delegation instead of adding individual 'focus' and 'blur' event listeners to each monitored element.
d408ec4
to
4513fb3
Compare
* perf(focus-monitor): optimize event registration Improve focus-monitor scalability by implementing event delegation instead of adding individual 'focus' and 'blur' event listeners to each monitored element. * Fix a bug in _getFocusOrigin(). * Add a comment explaining why we need to walk up the tree * Cleanup
In angular#18667 event delegation was implemented for the `FocusMonitor` which is based around binding a single event on the `document`. The problem is that the browser won't invoke the `focus` and `blur` handlers on the `document`, if focus is moved within the same shadow root. These changes switch to delegating the events either through the closest shadow root or the `document`. Fixes angular#19133.
In #18667 event delegation was implemented for the `FocusMonitor` which is based around binding a single event on the `document`. The problem is that the browser won't invoke the `focus` and `blur` handlers on the `document`, if focus is moved within the same shadow root. These changes switch to delegating the events either through the closest shadow root or the `document`. Fixes #19133.
In #18667 event delegation was implemented for the `FocusMonitor` which is based around binding a single event on the `document`. The problem is that the browser won't invoke the `focus` and `blur` handlers on the `document`, if focus is moved within the same shadow root. These changes switch to delegating the events either through the closest shadow root or the `document`. Fixes #19133.
In angular#18667 event delegation was implemented for the `FocusMonitor` which is based around binding a single event on the `document`. The problem is that the browser won't invoke the `focus` and `blur` handlers on the `document`, if focus is moved within the same shadow root. These changes switch to delegating the events either through the closest shadow root or the `document`. Fixes angular#19133.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Improve
FocusMonitor
scalability by implementing event delegation instead of adding individualfocus
andblur
event listeners to each monitored element.The global listeners do have to walk up the ancestor chain of event target in order to support
checkChildren
, but this is extremely fast even for very deeply nested DOM trees since all it has to do is callMap.has(element)
for each one.Another potential improvement may be to store the tracked elements in a
WeakMap
and remove the need to callstopMonitoring()
, making the API easier, but this would require always keeping the 6 global event listeners active. Right now, they are removed if there are no tracked elements.