Skip to content

Commit

Permalink
Fix: Add tabindex to preview container (#858)
Browse files Browse the repository at this point in the history
* Fix: Add tabindex to preview container

* Chore: Refactor for unit test

* Chore: addressing PR comments

* Chore: addressing PR comments

* Chore: adding tests to focusFullscreenElement

* Chore: adding back event callback test
  • Loading branch information
Conrad Chan authored Nov 6, 2018
1 parent 2040467 commit a37fdb7
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 13 deletions.
28 changes: 20 additions & 8 deletions src/lib/Fullscreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,27 +91,39 @@ class Fullscreen extends EventEmitter {
* Fires events when the fullscreen state changes
*
* @private
* @param {HTMLElement|Event} [el] - Fullscreen element
* @param {HTMLElement|Event} el - Fullscreen element
* @return {void}
*/
fullscreenchangeHandler = (el) => {
let enter = false;

if (this.isSupported()) {
if (this.isFullscreen()) {
enter = true;
}
} else if (!this.isFullscreen(el)) {
enter = true;
}
enter = (this.isSupported() && this.isFullscreen()) || (!this.isSupported() && !this.isFullscreen(el));

if (enter) {
this.focusFullscreenElement(el);
this.emit('enter');
} else {
this.emit('exit');
}
};

/**
* Focuses the element
*
* @private
* @param {HTMLElement|Event} el - Fullscreen element or event
* @return {void}
*/
focusFullscreenElement = (el) => {
// Focus on the fullscreen element so keyboard
// events are triggered without an extra click
// If el has target property, then it is an Event
// otherwise it is a HTMLElement
const element = el.target || el;

element.focus();
};

/**
* Toggles fullscreen mode
*
Expand Down
1 change: 1 addition & 0 deletions src/lib/__tests__/Fullscreen-test.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div id='test-container'></div>
45 changes: 41 additions & 4 deletions src/lib/__tests__/Fullscreen-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,30 +38,47 @@ describe('lib/Fullscreen', () => {
});

describe('fullscreenchangeHandler()', () => {
before(() => {
fixture.setBase('src/lib');
});

beforeEach(() => {
fixture.load('__tests__/Fullscreen-test.html');
});

afterEach(() => {
fixture.cleanup();
});

it('should emit enter if we are entering fullscreen and if true fullscreen is supported', () => {
sandbox.stub(fullscreen, 'isSupported').returns(true);
sandbox.stub(fullscreen, 'isFullscreen').returns(true);
sandbox.stub(fullscreen, 'emit');
sandbox.stub(fullscreen, 'focusFullscreenElement');

fullscreen.fullscreenchangeHandler({});

expect(fullscreen.emit).to.have.been.calledWith('enter');
expect(fullscreen.focusFullscreenElement.called).to.be.true;
});

it('should emit exit if we are exiting fullscreen and if true fullscreen is supported', () => {
sandbox.stub(fullscreen, 'isSupported').returns(true);
sandbox.stub(fullscreen, 'isFullscreen').returns(false);
sandbox.stub(fullscreen, 'emit');
sandbox.stub(fullscreen, 'focusFullscreenElement');

fullscreen.fullscreenchangeHandler({});

expect(fullscreen.emit).to.have.been.calledWith('exit');
expect(fullscreen.focusFullscreenElement.called).to.be.false;
});

it('should emit enter if we are entering fullscreen and if true fullscreen is not supported', () => {
sandbox.stub(fullscreen, 'isSupported').returns(false);
sandbox.stub(fullscreen, 'isFullscreen').returns(false);
sandbox.stub(fullscreen, 'emit');
sandbox.stub(fullscreen, 'focusFullscreenElement');

fullscreen.fullscreenchangeHandler({});

Expand All @@ -80,12 +97,11 @@ describe('lib/Fullscreen', () => {

it('should be called only once when the fullscreenchange event is emitted', () => {
const spy = sandbox.spy(fullscreen, 'fullscreenchangeHandler');
sandbox.stub(fullscreen, 'focusFullscreenElement').returns(true);
// rebind the dom listeners to use the spy
fullscreen.bindDOMListeners();

const event = new Event('webkitfullscreenchange');

window.document.dispatchEvent(event);
const event = new Event('webkitfullscreenchange', { bubbles: true });
document.getElementById('test-container').dispatchEvent(event);
expect(spy).to.be.called.once;
});
});
Expand Down Expand Up @@ -213,4 +229,25 @@ describe('lib/Fullscreen', () => {
expect(fullscreen.fullscreenchangeHandler).to.have.been.calledWith(element);
});
});

describe('focusFullscreenElement()', () => {
it('should focus the element when element passed in', () => {
const element = document.createElement('div');
sandbox.stub(element, 'focus');

fullscreen.focusFullscreenElement(element);

expect(element.focus.called).to.be.true;
});

it('should focus the element when event is passed in', () => {
const element = document.createElement('div');
sandbox.stub(element, 'focus');
const event = { target: element };

fullscreen.focusFullscreenElement(event);

expect(element.focus.called).to.be.true;
});
});
});
2 changes: 1 addition & 1 deletion src/lib/shell.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
</div>
</div>
</div>
<div class="bp">
<div class="bp" tabindex="0">
<div class="bp-loading-wrapper">
<div class="bp-loading">
<div class="bp-icon bp-icon-file"></div>
Expand Down

0 comments on commit a37fdb7

Please sign in to comment.