Skip to content
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

Add keycodes utility #9004

Merged
merged 1 commit into from Apr 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions extensions/amp-image-lightbox/0.1/amp-image-lightbox.js
Expand Up @@ -16,13 +16,14 @@

import {Animation} from '../../../src/animation';
import {CSS} from '../../../build/amp-image-lightbox-0.1.css';
import {Gestures} from '../../../src/gesture';
import {
DoubletapRecognizer,
SwipeXYRecognizer,
TapRecognizer,
TapzoomRecognizer,
} from '../../../src/gesture-recognizers';
import {Gestures} from '../../../src/gesture';
import {Keycodes} from '../../../src/utils/keycodes';
import {Layout} from '../../../src/layout';
import {bezierCurve} from '../../../src/curve';
import {continueMotion} from '../../../src/motion';
Expand Down Expand Up @@ -824,7 +825,7 @@ class AmpImageLightbox extends AMP.BaseElement {
* @private
*/
closeOnEscape_(event) {
if (event.keyCode == 27) {
if (event.keyCode == Keycodes.ESCAPE) {
this.close();
}
}
Expand Down
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import {Keycodes} from '../../../../src/utils/keycodes';
import {timerFor} from '../../../../src/services';
import {createIframePromise} from '../../../../testing/iframe';
import '../amp-image-lightbox';
Expand Down Expand Up @@ -199,7 +200,7 @@ describe('amp-image-lightbox component', () => {
ampImage.setAttribute('width', '100');
ampImage.setAttribute('height', '100');
impl.activate({source: ampImage});
impl.closeOnEscape_({keyCode: 27});
impl.closeOnEscape_({keyCode: Keycodes.ESCAPE});
expect(setupCloseSpy).to.be.calledOnce;

// Regression test: ensure escape event listener is bound properly
Expand Down
18 changes: 5 additions & 13 deletions extensions/amp-lightbox-viewer/0.1/amp-lightbox-viewer.js
Expand Up @@ -16,6 +16,7 @@


import {CSS} from '../../../build/amp-lightbox-viewer-0.1.css';
import {Keycodes} from '../../../src/utils/keycodes';
import {ampdocServiceFor} from '../../../src/ampdoc';
import {ancestorElements} from '../../../src/dom';
import {isExperimentOn} from '../../../src/experiments';
Expand Down Expand Up @@ -422,22 +423,13 @@ export class AmpLightboxViewer extends AMP.BaseElement {
* @private
*/
handleKeyboardEvents_(event) {
// TODO(aghassemi): Add helper utility for keyboard events or an enum.
// TODO(aghassemi): RTL support
const code = event.keyCode;

// Escape
if (code == 27) {
if (code == Keycodes.ESCAPE) {
this.close_();
}

// TODO(aghassemi): RTL support
// Right arrow
if (code == 39) {
} else if (code == Keycodes.RIGHT_ARROW) {
this.next_();
}

// Left arrow
if (code == 37) {
} else if (code == Keycodes.LEFT_ARROW) {
this.previous_();
}
}
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-lightbox/0.1/amp-lightbox.js
Expand Up @@ -16,6 +16,7 @@

import {CSS} from '../../../build/amp-lightbox-0.1.css';
import {Gestures} from '../../../src/gesture';
import {Keycodes} from '../../../src/utils/keycodes';
import {Layout} from '../../../src/layout';
import {SwipeXYRecognizer} from '../../../src/gesture-recognizers';
import {childElementByTag} from '../../../src/dom.js';
Expand Down Expand Up @@ -300,7 +301,7 @@ class AmpLightbox extends AMP.BaseElement {
* @private
*/
closeOnEscape_(event) {
if (event.keyCode == 27) {
if (event.keyCode == Keycodes.ESCAPE) {
this.close();
}
}
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-sidebar/0.1/amp-sidebar.js
Expand Up @@ -15,6 +15,7 @@
*/

import {CSS} from '../../../build/amp-sidebar-0.1.css';
import {Keycodes} from '../../../src/utils/keycodes';
import {closestByTag, tryFocus} from '../../../src/dom';
import {Layout} from '../../../src/layout';
import {dev} from '../../../src/log';
Expand Down Expand Up @@ -112,7 +113,7 @@ export class AmpSidebar extends AMP.BaseElement {

this.documentElement_.addEventListener('keydown', event => {
// Close sidebar on ESC.
if (event.keyCode == 27) {
if (event.keyCode == Keycodes.ESCAPE) {
this.close_();
}
});
Expand Down
5 changes: 3 additions & 2 deletions extensions/amp-sidebar/0.1/test/test-amp-sidebar.js
Expand Up @@ -15,6 +15,7 @@
* limitations under the License.
*/

import {Keycodes} from '../../../../src/utils/keycodes';
import {adopt} from '../../../../src/runtime';
import {createIframePromise} from '../../../../testing/iframe';
import {platformFor} from '../../../../src/services';
Expand Down Expand Up @@ -270,8 +271,8 @@ describe('amp-sidebar', () => {
if (eventObj.initEvent) {
eventObj.initEvent('keydown', true, true);
}
eventObj.keyCode = 27;
eventObj.which = 27;
eventObj.keyCode = Keycodes.ESCAPE;
eventObj.which = Keycodes.ESCAPE;
const el = iframe.doc.documentElement;
el.dispatchEvent ?
el.dispatchEvent(eventObj) : el.fireEvent('onkeydown', eventObj);
Expand Down
27 changes: 27 additions & 0 deletions src/utils/keycodes.js
@@ -0,0 +1,27 @@
/**
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @enum {number}
*/
export const Keycodes = {
ESCAPE: 27,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESC instead of ESCAPE?

SPACE: 32,
LEFT_ARROW: 37,
UP_ARROW: 38,
RIGHT_ARROW: 39,
DOWN_ARROW: 40,
};