-
Notifications
You must be signed in to change notification settings - Fork 6.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
fix(MdCheckbox): Cancel spacebar keydown events when component focused #181
Conversation
@@ -192,6 +199,15 @@ export class MdCheckbox implements ControlValueAccessor { | |||
this.toggle(); | |||
} | |||
|
|||
onFocus(evt: Event) { | |||
window.addEventListener('keydown', this._focusWindowKeydownHandler); |
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.
There's actually a special syntax for doing this without directly touching window:
host: {
'(window:domEvent)': 'onWindowEvent($event.type)',
'(document:domEvent)': 'onDocumentEvent($event.type)',
'(body:domEvent)': 'onBodyEvent($event.type)'
}
So you can do 'window:focus': 'myFocusHandler()'
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.
ah that's way better. will change.
4d9c263
to
fcee873
Compare
@jelbourn changes made! |
* from bubbling when the component is focused, which prevents side effects like page scrolling | ||
* from happening. | ||
*/ | ||
onWindowKeyDown(evt: KeyboardEvent) { |
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.
As discussed in chat: just use keydown
on the host element since it should only fire when it has focus anyway?
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.
👍 yeah I think I overthought it. Changed.
This commit ensures that when a checkbox element is focused, spacebar `keydown` events will not bubble up to window, causing side effects like page scrolling. This commit also fixes an issue as part of this change where the component could still be focused even when it was disabled. Finally, some of the test code has been cleaned up and corrected. Fixes angular#162
fcee873
to
ac84e25
Compare
LGTM |
* Adds the `--depth` option when cloning the assets for the docs.
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. |
This commit ensures that when a checkbox element is focused, spacebar
keydown
events will not bubble up to window, causing side effects likepage scrolling.
This commit also fixes an issue as part of this change where the
component could still be focused even when it was disabled.
Finally, some of the test code has been cleaned up and corrected.
Fixes #162