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.

Sign up for GitHub

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

Jump to bottom

Inverts arrow key focus movement when in an RTL context. #41

Merged
merged 6 commits into from
Feb 2, 2016
Merged

Inverts arrow key focus movement when in an RTL context. #41

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f72ff76
Check for RTL when changing selection on directional key press.
bicknellr Nov 6, 2015
7dccc86
Use `getComputedStyle` to determine direction.
bicknellr Nov 6, 2015
8fb7c1a
Test directional key behavior in RTL.
bicknellr Nov 6, 2015
befda3c
Bump Polymer dep to ^1.2.4 for activeElement use in tests; test that …
bicknellr Jan 27, 2016
f67e49d
Remove `setTimeout` calls from synchronous tests.
bicknellr Feb 2, 2016
525d515
Remove call to `done` now that the tests are sync.
bicknellr Feb 2, 2016
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
2 changes: 1 addition & 1 deletion bower.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"ignore": [],
"dependencies": {
"iron-selector": "PolymerElements/iron-selector#^1.0.0",
"polymer": "Polymer/polymer#^1.0.0",
"polymer": "Polymer/polymer#^1.2.4",
"iron-a11y-keys-behavior": "polymerelements/iron-a11y-keys-behavior#^1.0.0"
},
"devDependencies": {
Expand Down
16 changes: 14 additions & 2 deletions iron-menubar-behavior.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,24 @@
event.detail.keyboardEvent.preventDefault();
},

get _isRTL() {
return window.getComputedStyle(this)['direction'] === 'rtl';
},

_onLeftKey: function() {
this._focusPrevious();
if (this._isRTL) {
this._focusNext();
} else {
this._focusPrevious();
}
},

_onRightKey: function() {
this._focusNext();
if (this._isRTL) {
this._focusPrevious();
} else {
this._focusNext();
}
},

_onKeydown: function(event) {
Expand Down
65 changes: 60 additions & 5 deletions test/iron-menubar-behavior.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

<script src="../../webcomponentsjs/webcomponents-lite.js"></script>
<script src="../../web-component-tester/browser.js"></script>
<script src="../../iron-test-helpers/mock-interactions.js"></script>
<link rel="import" href="../../iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="test-menubar.html">

</head>
Expand All @@ -45,6 +45,18 @@
</template>
</test-fixture>

<test-fixture id="rtl">
<template>
<div dir="rtl">
<test-menubar>
<div>item 1</div>
<div>item 2</div>
<div>item 3</div>
</test-menubar>
</div>
</template>
</test-fixture>

<script>

suite('menubar a11y tests', function() {
Expand All @@ -58,7 +70,7 @@
var menubar = fixture('basic');
MockInteractions.focus(menubar);
setTimeout(function() {
assert.equal(document.activeElement, menubar.firstElementChild, 'document.activeElement is first item')
assert.equal(Polymer.dom(document).activeElement, menubar.firstElementChild, 'document.activeElement is first item')
done();
// wait for async in _onFocus
}, 200);
Expand All @@ -69,7 +81,7 @@
menubar.selected = 1;
MockInteractions.focus(menubar);
setTimeout(function() {
assert.equal(document.activeElement, menubar.selectedItem, 'document.activeElement is selected item');
assert.equal(Polymer.dom(document).activeElement, menubar.selectedItem, 'document.activeElement is selected item');
done();
// wait for async in _onFocus
}, 200);
Expand All @@ -80,7 +92,7 @@
menubar.selected = 0;
menubar.items[1].click();
setTimeout(function() {
assert.equal(document.activeElement, menubar.items[1], 'document.activeElement is last activated item');
assert.equal(Polymer.dom(document).activeElement, menubar.items[1], 'document.activeElement is last activated item');
done();
// wait for async in _onFocus
}, 200);
Expand All @@ -91,12 +103,55 @@
menubar.selected = 0;
menubar.items[0].click();
setTimeout(function() {
assert.equal(document.activeElement, menubar.items[0], 'document.activeElement is last activated item');
assert.equal(Polymer.dom(document).activeElement, menubar.items[0], 'document.activeElement is last activated item');
done();
// wait for async in _onFocus
}, 200);
});

suite('left / right keys are reversed when the menubar has RTL directionality', function() {
var LEFT = 37;
var RIGHT = 39;

test('left key moves to the next item', function(done) {
var rtlContainer = fixture('rtl');
var menubar = rtlContainer.querySelector('test-menubar');
menubar.selected = 0;
menubar.items[1].click();
setTimeout(function() {
MockInteractions.pressAndReleaseKeyOn(menubar, LEFT);
setTimeout(function() {
assert.equal(Polymer.dom(document).activeElement, menubar.items[2],
'`document.activeElement` should be the next item.');
assert.equal(menubar.selected, 1,
'`menubar.selected` should not change.');
done();
// wait for async in _onFocus
}, 200);
Copy link
Contributor

Choose a reason for hiding this comment

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

why 200 and not 100, or 1? it is waiting for an animation to finish, or just a microtask?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I copied this from the other examples with the assumption that both actions would need to wait for the same thing because they both affect focus, so I wouldn't be surprised if the timing is too loose. I'll look more into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(This comment is still applicable.)

// wait for async in _onFocus
}, 200);
});

test('right key moves to the previous item', function(done) {
var rtlContainer = fixture('rtl');
var menubar = rtlContainer.querySelector('test-menubar');
menubar.selected = 0;
menubar.items[1].click();
setTimeout(function() {
MockInteractions.pressAndReleaseKeyOn(menubar, RIGHT);
setTimeout(function() {
assert.equal(Polymer.dom(document).activeElement, menubar.items[0],
'`document.activeElement` should be the previous item');
assert.equal(menubar.selected, 1,
'`menubar.selected` should not change.');
done();
// wait for async in _onFocus
}, 200);
// wait for async in _onFocus
}, 200);
});
});

});

</script>
Expand Down