feat(focus-classes): expose focus origin changes through observable #2974
Conversation
googlebot
added
the
cla: yes
src/lib/core/style/focus-classes.ts
Outdated
| renderer.setElementClass(element, 'cdk-program-focused', this._origin == 'program'); | ||
|
|
||
| subject.next(this._origin); | ||
| this._lastFocused = new WeakMap().set(element, this._origin); |
There was a problem hiding this comment.
I'm trying to understand why we need a WeakMap when the the WeakMap always contains one element at most.
I think we can assume that the browser always returns focus to the last focused element and multiple elements can't be focused.
There was a problem hiding this comment.
I guess I was thinking I needed a reference to the element to check that it was the same one, but you're probably right that I can safely assume that. Let me try.
| renderer.setElementClass(element, 'cdk-focused', false); | ||
| renderer.setElementClass(element, 'cdk-keyboard-focused', false); | ||
| renderer.setElementClass(element, 'cdk-mouse-focused', false); | ||
| renderer.setElementClass(element, 'cdk-program-focused', false); | ||
| subject.next(null); |
There was a problem hiding this comment.
Do we really want to emit null on blur? I didn't see any test for it yet.
There was a problem hiding this comment.
I do think we want this, because I want to later expand this to apply the classes if the element or one of its children is focused, and it would be nice to have an easy way for developers to know when focus leaves the subtree. I'll add tests though
There was a problem hiding this comment.
Sounds good. Just was wondering because of the tests.
|
updated, PTAL |
|
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. |
also has a fix to restore focus origin after window loses focus and then regains it