Commit 7e3da6dc authored by sangwoo.ko's avatar sangwoo.ko Committed by Commit Bot

Introduce consume-keydown-event property to cr_dialog

For cr_dialog to behave modaaly, it's better to stop propagation of
'keydown' events. To support this, introduce consume-keydown-event
to cr_dialog. If this property is true, cr_dialog will consume
keydown events.


Bug: 883221
Change-Id: If2981794cad08dfa6dbca8e08d2d16351fad47bd
Reviewed-on: https://chromium-review.googlesource.com/c/1254001
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Reviewed-by: default avatarcalamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597039}
parent e14824d2
...@@ -746,6 +746,7 @@ Sanghee Lee <sanghee.lee1992@gmail.com> ...@@ -746,6 +746,7 @@ Sanghee Lee <sanghee.lee1992@gmail.com>
Sanghyun Park <sh919.park@samsung.com> Sanghyun Park <sh919.park@samsung.com>
Sanghyup Lee <sh53.lee@samsung.com> Sanghyup Lee <sh53.lee@samsung.com>
Sangjoon Je <htamop@gmail.com> Sangjoon Je <htamop@gmail.com>
Sangwoo Ko <sangwoo.ko@navercorp.com>
Sangwoo Ko <sangwoo108@gmail.com> Sangwoo Ko <sangwoo108@gmail.com>
Sanjoy Pal <ncj674@motorola.com> Sanjoy Pal <ncj674@motorola.com>
Sanjoy Pal <sanjoy.pal@samsung.com> Sanjoy Pal <sanjoy.pal@samsung.com>
......
...@@ -60,7 +60,7 @@ ...@@ -60,7 +60,7 @@
<cr-lazy-render id="dialog"> <cr-lazy-render id="dialog">
<template> <template>
<cr-dialog> <cr-dialog consume-keydown-event>
<div slot="title">$i18n{removeSelected}</div> <div slot="title">$i18n{removeSelected}</div>
<div slot="body">$i18n{deleteWarning}</div> <div slot="body">$i18n{deleteWarning}</div>
<div slot="button-container"> <div slot="button-container">
......
...@@ -274,7 +274,7 @@ suite('cr-dialog', function() { ...@@ -274,7 +274,7 @@ suite('cr-dialog', function() {
// Ensuring that intersectionObserver does not fire any callbacks before the // Ensuring that intersectionObserver does not fire any callbacks before the
// dialog has been opened. // dialog has been opened.
test('body scrollable border not added before modal shown', function(done) { test('body scrollable border not added before modal shown', function() {
document.body.innerHTML = ` document.body.innerHTML = `
<cr-dialog> <cr-dialog>
<div slot="title">title</div> <div slot="title">title</div>
...@@ -286,13 +286,10 @@ suite('cr-dialog', function() { ...@@ -286,13 +286,10 @@ suite('cr-dialog', function() {
const bodyContainer = dialog.$$('.body-container'); const bodyContainer = dialog.$$('.body-container');
assertTrue(!!bodyContainer); assertTrue(!!bodyContainer);
// Waiting for 1ms because IntersectionObserver fires one message loop after return PolymerTest.flushTasks().then(() => {
// dialog.attached.
setTimeout(function() {
assertFalse(bodyContainer.classList.contains('top-scrollable')); assertFalse(bodyContainer.classList.contains('top-scrollable'));
assertFalse(bodyContainer.classList.contains('bottom-scrollable')); assertFalse(bodyContainer.classList.contains('bottom-scrollable'));
done(); });
}, 1);
}); });
test('dialog body scrollable border when appropriate', function(done) { test('dialog body scrollable border when appropriate', function(done) {
...@@ -427,4 +424,51 @@ suite('cr-dialog', function() { ...@@ -427,4 +424,51 @@ suite('cr-dialog', function() {
'none', 'none',
window.getComputedStyle(dialog.getCloseButton().parentElement).display); window.getComputedStyle(dialog.getCloseButton().parentElement).display);
}); });
test('keydown should be consumed when the property is true', function() {
document.body.innerHTML = `
<cr-dialog consume-keydown-event>
<div slot="title">title</div>
</cr-dialog>`;
const dialog = document.body.querySelector('cr-dialog');
dialog.showModal();
assertTrue(dialog.open);
assertTrue(dialog.consumeKeydownEvent);
function assertKeydownNotReached() {
assertNotReached('keydown event was propagated');
}
document.addEventListener('keydown', assertKeydownNotReached);
return PolymerTest.flushTasks().then(() => {
MockInteractions.keyDownOn(dialog, 65, undefined, 'a');
MockInteractions.keyDownOn(document.body, 65, undefined, 'a');
document.removeEventListener('keydown', assertKeydownNotReached);
});
});
test('keydown should be propagated when the property is false', function() {
document.body.innerHTML = `
<cr-dialog>
<div slot="title">title</div>
</cr-dialog>`;
const dialog = document.body.querySelector('cr-dialog');
dialog.showModal();
assertTrue(dialog.open);
assertFalse(dialog.consumeKeydownEvent);
let keydownCounter = 0;
function assertKeydownCount() {
keydownCounter++;
}
document.addEventListener('keydown', assertKeydownCount);
return PolymerTest.flushTasks().then(() => {
MockInteractions.keyDownOn(dialog, 65, undefined, 'a');
assertEquals(1, keydownCounter);
document.removeEventListener('keydown', assertKeydownCount);
});
});
}); });
...@@ -51,6 +51,15 @@ Polymer({ ...@@ -51,6 +51,15 @@ Polymer({
value: false, value: false,
}, },
/**
* True if the dialog should consume 'keydown' events. If ignoreEnterKey
* is true, 'Enter' key won't be consumed.
*/
consumeKeydownEvent: {
type: Boolean,
value: false,
},
/** /**
* True if the dialog should not be able to be cancelled, which will prevent * True if the dialog should not be able to be cancelled, which will prevent
* 'Escape' key presses from closing the dialog. * 'Escape' key presses from closing the dialog.
...@@ -77,6 +86,9 @@ Polymer({ ...@@ -77,6 +86,9 @@ Polymer({
/** @private {?MutationObserver} */ /** @private {?MutationObserver} */
mutationObserver_: null, mutationObserver_: null,
/** @private {?Function} */
boundKeydown_: null,
/** @override */ /** @override */
ready: function() { ready: function() {
// If the active history entry changes (i.e. user clicks back button), // If the active history entry changes (i.e. user clicks back button),
...@@ -93,10 +105,13 @@ Polymer({ ...@@ -93,10 +105,13 @@ Polymer({
/** @override */ /** @override */
attached: function() { attached: function() {
var mutationObserverCallback = function() { var mutationObserverCallback = function() {
if (this.$.dialog.open) if (this.$.dialog.open) {
this.addIntersectionObserver_(); this.addIntersectionObserver_();
else this.addKeydownListener_();
} else {
this.removeIntersectionObserver_(); this.removeIntersectionObserver_();
this.removeKeydownListener_();
}
}.bind(this); }.bind(this);
this.mutationObserver_ = new MutationObserver(mutationObserverCallback); this.mutationObserver_ = new MutationObserver(mutationObserverCallback);
...@@ -113,6 +128,7 @@ Polymer({ ...@@ -113,6 +128,7 @@ Polymer({
/** @override */ /** @override */
detached: function() { detached: function() {
this.removeIntersectionObserver_(); this.removeIntersectionObserver_();
this.removeKeydownListener_();
if (this.mutationObserver_) { if (this.mutationObserver_) {
this.mutationObserver_.disconnect(); this.mutationObserver_.disconnect();
this.mutationObserver_ = null; this.mutationObserver_ = null;
...@@ -163,6 +179,31 @@ Polymer({ ...@@ -163,6 +179,31 @@ Polymer({
} }
}, },
/** @private */
addKeydownListener_: function() {
if (!this.consumeKeydownEvent)
return;
this.boundKeydown_ = this.boundKeydown_ || this.onKeydown_.bind(this);
this.addEventListener('keydown', this.boundKeydown_);
// Sometimes <body> is key event's target and in that case the event
// will bypass cr-dialog. We should consume those events too in order to
// behave modally. This prevents accidentally triggering keyboard commands.
document.body.addEventListener('keydown', this.boundKeydown_);
},
/** @private */
removeKeydownListener_: function() {
if (!this.boundKeydown_)
return;
this.removeEventListener('keydown', this.boundKeydown_);
document.body.removeEventListener('keydown', this.boundKeydown_);
this.boundKeydown_ = null;
},
showModal: function() { showModal: function() {
this.$.dialog.showModal(); this.$.dialog.showModal();
assert(this.$.dialog.open); assert(this.$.dialog.open);
...@@ -269,6 +310,23 @@ Polymer({ ...@@ -269,6 +310,23 @@ Polymer({
} }
}, },
/**
* @param {!Event} e
* @private
*/
onKeydown_: function(e) {
assert(this.consumeKeydownEvent);
if (!this.getNative().open)
return;
if (this.ignoreEnterKey && e.key == 'Enter')
return;
// Stop propagation to behave modally.
e.stopPropagation();
},
/** @param {!PointerEvent} e */ /** @param {!PointerEvent} e */
onPointerdown_: function(e) { onPointerdown_: function(e) {
// Only show pulse animation if user left-clicked outside of the dialog // Only show pulse animation if user left-clicked outside of the dialog
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment