LAZR Javascript Library

Merge lp:~jtv/lazr-js/bug-480986 into lp:lazr-js

  1. bug-480986
  2. Merge into toolchain
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Barry Warsaw
Approved revision: 135
Merged at revision: not available
Proposed branch: lp:~jtv/lazr-js/bug-480986
Merge into: lp:lazr-js
Diff against target: 110 lines (+17/-21)
2 files modified
src-js/lazrjs/overlay/overlay.js (+3/-4)
src-js/lazrjs/overlay/tests/overlay.js (+14/-17)
To merge this branch: bzr merge lp:~jtv/lazr-js/bug-480986
Reviewer Review Type Date Requested Status
Barry Warsaw (community) code Approve
Review via email: mp+14801@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= Bug 480986 =

The Picker's escape handling wasn't working in Chrome. The reason is this Chrome issue: http://www.google.ru/support/forum/p/Chrome/thread?tid=2c74b15190723cf6&hl=en

In a nutshell, Chrome doesn't do keypress events for the escape key. This was not affecting the autocomplete widget or the inline editor since they listen for "key" events (and obey the escape right when you press it rather than when you release it). But it did affect the other overlay-based widgets since it was in the overlay that the escape key was handled. To reproduce, open any of these widgets (choiceedit, formoverlay, overlay, or picker) in Chrome or Chromium and press escape. It should close the widget, but with the bug still present it will do nothing.

In this branch, the overlay's escape handling is replaced with the inline editor's approach. I discussed with the author of the original escape handling in the overlay and we found that this was the best approach after all.

You'll also find a few drive-by cleanups prompted by "make lint."

Jeroen

Revision history for this message
Barry Warsaw (barry) wrote :

As discussed:

* reformat line 13-14
* add comment for down:27

review: Approve (code)
lp:~jtv/lazr-js/bug-480986 updated
136. By Jeroen T. Vermeulen

Review change.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src-js/lazrjs/overlay/overlay.js'
2--- src-js/lazrjs/overlay/overlay.js 2009-10-23 18:24:21 +0000
3+++ src-js/lazrjs/overlay/overlay.js 2009-11-12 20:53:09 +0000
4@@ -209,11 +209,10 @@
5 var visible = this.get('visible');
6 if (visible) {
7 Y.get('body').appendChild(this._blocking_div);
8- this._doc_kp_handler = Y.on('keypress', function(e) {
9- if (e.keyCode == ESCAPE) {
10+ // Handle Escape (code 27) on keydown.
11+ this._doc_kp_handler = Y.on('key', function() {
12 self.fire(CANCEL);
13- }
14- }, document);
15+ }, document, 'down:27');
16 } else {
17 this._removeBlockingDiv();
18 }
19
20=== modified file 'src-js/lazrjs/overlay/tests/overlay.js'
21--- src-js/lazrjs/overlay/tests/overlay.js 2009-10-22 18:37:47 +0000
22+++ src-js/lazrjs/overlay/tests/overlay.js 2009-11-12 20:53:09 +0000
23@@ -51,6 +51,12 @@
24 cleanup_widget(this.overlay);
25 },
26
27+ hitEscape: function() {
28+ simulate(this.overlay.get('boundingBox'),
29+ '.close .close-button',
30+ 'keydown', { keyCode: ESCAPE });
31+ },
32+
33 test_picker_can_be_instantiated: function() {
34 this.overlay = new Y.lazr.PrettyOverlay();
35 Assert.isInstanceOf(
36@@ -146,9 +152,7 @@
37 this.overlay = new Y.lazr.PrettyOverlay();
38 this.overlay.render();
39
40- simulate(this.overlay.get('boundingBox'),
41- '.close .close-button',
42- 'keypress', { keyCode: ESCAPE });
43+ this.hitEscape();
44 Assert.isFalse(this.overlay.get('visible'), "The widget wasn't hidden");
45 },
46
47@@ -162,9 +166,7 @@
48 event_was_fired = true;
49 });
50 }, this);
51- simulate(this.overlay.get('boundingBox'),
52- '.close .close-button',
53- 'click', { keyCode: ESCAPE });
54+ this.hitEscape();
55 this.wait(function() {
56 Assert.isTrue(event_was_fired, "cancel event wasn't fired");
57 }, 3000);
58@@ -177,15 +179,11 @@
59 this.overlay = new Y.lazr.PrettyOverlay();
60 this.overlay.render();
61
62- simulate(this.overlay.get('boundingBox'),
63- '.close .close-button',
64- 'keypress', { keyCode: ESCAPE });
65+ this.hitEscape();
66 Assert.isFalse(this.overlay.get('visible'), "The widget wasn't hidden");
67 this.overlay.show();
68 Assert.isTrue(this.overlay.get('visible'), "The widget wasn't shown again");
69- simulate(this.overlay.get('boundingBox'),
70- '.close .close-button',
71- 'keypress', { keyCode: ESCAPE });
72+ this.hitEscape();
73 Assert.isFalse(this.overlay.get('visible'), "The widget wasn't hidden");
74 },
75
76@@ -193,7 +191,7 @@
77 this.overlay = new Y.lazr.PrettyOverlay();
78 function PrettyOverlaySubclass(config) {
79 PrettyOverlaySubclass.superclass.constructor.apply(this, arguments);
80- };
81+ }
82 PrettyOverlaySubclass.NAME = 'lazr-overlaysubclass';
83 Y.extend(PrettyOverlaySubclass, Y.lazr.PrettyOverlay);
84
85@@ -206,14 +204,13 @@
86 test_overlay_bodyContent_has_size_1: function() {
87 var overlay = new Y.Overlay({
88 headerContent: 'Form for testing',
89- bodyContent: '<input type="text" name="field1" />',
90+ bodyContent: '<input type="text" name="field1" />'
91 });
92 overlay.render();
93 Assert.areEqual(
94 1,
95 overlay.get("bodyContent").size(),
96- "The bodContent should contain only one node."
97- )
98+ "The bodContent should contain only one node.");
99 },
100
101 test_set_progress: function() {
102@@ -226,7 +223,7 @@
103 Assert.areEqual(
104 '23%',
105 this.overlay.get('boundingBox').query('.steps .step-on').getStyle('width')
106- )
107+ );
108 }
109
110 }));

Subscribers

People subscribed via source and target branches