Skip to content

Commit 129a704

Browse files
celiolatorracaraphamorim
authored andcommitted
Refact with-focusable to make focusPath optional and use DOMElement as default
1 parent ae6594f commit 129a704

File tree

5 files changed

+66
-23
lines changed

5 files changed

+66
-23
lines changed

__tests__/spatial-navigation-test.js

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import SpatialNavigation from '../src/spatial-navigation';
22

33
describe('SpatialNavigation', () => {
4-
describe('initialize', () => {
5-
let setStateSpy;
4+
let setStateSpy;
65

7-
beforeEach(() => {
8-
setStateSpy = jest.fn();
9-
SpatialNavigation.init(setStateSpy);
10-
});
6+
beforeEach(() => {
7+
setStateSpy = jest.fn();
8+
SpatialNavigation.init(setStateSpy);
9+
});
1110

11+
afterEach(() => {
12+
SpatialNavigation.destroy();
13+
});
14+
15+
describe('on initialize', () => {
1216
it('listens to sn:focused event', () => {
1317
const event = new CustomEvent('sn:focused', {
1418
detail: { sectionId: 'focusPath' },
@@ -20,7 +24,7 @@ describe('SpatialNavigation', () => {
2024

2125
describe('when focusing the same focused element', () => {
2226
beforeEach(() => {
23-
SpatialNavigation.focused = 'focusPath';
27+
SpatialNavigation.focusedPath = 'focusPath';
2428
});
2529

2630
it('does nothing', () => {
@@ -34,7 +38,16 @@ describe('SpatialNavigation', () => {
3438
});
3539
});
3640

37-
describe('destroy', () => {
41+
describe('on destroy', () => {
42+
it('stops listening to sn:focused', () => {
43+
SpatialNavigation.destroy();
44+
45+
const event = new CustomEvent('sn:focused', {
46+
detail: { sectionId: 'focusPath' },
47+
});
48+
document.dispatchEvent(event);
3849

50+
expect(setStateSpy).not.toHaveBeenCalled();
51+
});
3952
});
4053
});

__tests__/with-focusable-test.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ describe('withFocusable', () => {
2323

2424
let component;
2525

26+
afterEach(() => {
27+
if (component) {
28+
component.unmount();
29+
}
30+
});
31+
2632
it('injects focusPath as prop', () => {
2733
component = renderComponent({ focusPath: 'focusPath' });
2834
expect(component.find(Component).prop('focusPath')).toEqual('focusPath');
@@ -97,8 +103,8 @@ describe('withFocusable', () => {
97103
component = renderComponent({ focusPath: 'focusPath' });
98104

99105
component.unmount();
100-
expect(SpatialNavigation.removeFocusable)
101-
.toHaveBeenCalledWith('focusPath');
106+
// TODO: back toHaveBeenCalledWith, testing DOM Element
107+
expect(SpatialNavigation.removeFocusable).toHaveBeenCalled();
102108
});
103109
});
104110
});

src/navigation.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1183,7 +1183,9 @@ var GlobalConfig = {
11831183
} else {
11841184
_defaultSectionId = sectionId;
11851185
}
1186-
}
1186+
},
1187+
1188+
getSectionId: getSectionId
11871189
};
11881190

11891191
export default SpatialNavigation

src/spatial-navigation.js

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@ import Navigation from './navigation';
22

33
class SpatialNavigation {
44
constructor() {
5-
this.focusedPath = null;
6-
this.setState = null;
7-
85
this.handleFocused = this.handleFocused.bind(this);
9-
document.addEventListener('sn:focused', this.handleFocused);
6+
7+
this.destroy();
8+
this.bindFocusEvent();
109
}
1110

1211
init(updateState) {
@@ -16,12 +15,27 @@ class SpatialNavigation {
1615

1716
Navigation.init();
1817
Navigation.focus();
18+
this.bindFocusEvent();
1919
}
2020

2121
destroy() {
22+
this.focusedPath = null;
2223
this.setState = null;
24+
2325
Navigation.uninit();
26+
this.unbindFocusEvent();
27+
}
28+
29+
bindFocusEvent() {
30+
if (!this.listening) {
31+
this.listening = true;
32+
document.addEventListener('sn:focused', this.handleFocused);
33+
}
34+
}
35+
36+
unbindFocusEvent() {
2437
document.removeEventListener('sn:focused', this.handleFocused);
38+
this.listening = false;
2539
}
2640

2741
handleFocused(ev) {
@@ -40,15 +54,23 @@ class SpatialNavigation {
4054
Navigation.focus(focusPath);
4155
}
4256

43-
addFocusable(focusPath, focusDOMElement) {
44-
this.removeFocusable(focusPath);
57+
addFocusable(focusDOMElement, focusPath) {
58+
this.removeFocusable(focusDOMElement);
4559

46-
Navigation.add(focusPath, {selector: focusDOMElement});
47-
Navigation.makeFocusable(focusPath);
60+
const params = [{ selector: focusDOMElement }];
61+
if (focusPath) {
62+
params.unshift(focusPath);
63+
}
64+
65+
Navigation.add(...params);
66+
Navigation.makeFocusable(Navigation.getSectionId(focusDOMElement));
4867
}
4968

50-
removeFocusable(focusPath) {
51-
Navigation.remove(focusPath);
69+
removeFocusable(focusDOMElement) {
70+
const sectionId = Navigation.getSectionId(focusDOMElement);
71+
if (sectionId) {
72+
Navigation.remove(sectionId);
73+
}
5274
}
5375
}
5476

src/with-focusable.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ const withFocusable = ({
2323
})),
2424
lifecycle({
2525
componentDidMount() {
26-
SpatialNavigation.addFocusable(focusPath, ReactTV.findDOMNode(this));
26+
SpatialNavigation.addFocusable(ReactTV.findDOMNode(this), focusPath);
2727
},
2828
componentWillUnmount() {
29-
SpatialNavigation.removeFocusable(focusPath);
29+
SpatialNavigation.removeFocusable(ReactTV.findDOMNode(this));
3030
},
3131
}),
3232
);

0 commit comments

Comments
 (0)