Unverified Commit 86ff1955 authored by BM's avatar BM Committed by GitHub
Browse files

fix(Carousel): support for ref attribute (#4917)



* fix(Carousel): support for ref attribute

* fix(Carousel): revisions to ref attribute

* fix(Carousel): add imperative logic to expose refs

* refactor: Revise implementation to use hooks more

* simplify

* fix callbacks

* Apply suggestions from code review
Co-authored-by: default avatarmuzakparov <b.muzakparov@newagesol.com>
Co-authored-by: default avatarJimmy Jia <tesrin@gmail.com>
parent fac2160c
...@@ -65,7 +65,7 @@ ...@@ -65,7 +65,7 @@
"dependencies": { "dependencies": {
"@babel/runtime": "^7.4.2", "@babel/runtime": "^7.4.2",
"@restart/context": "^2.1.4", "@restart/context": "^2.1.4",
"@restart/hooks": "^0.3.11", "@restart/hooks": "^0.3.21",
"@types/react": "^16.8.23", "@types/react": "^16.8.23",
"classnames": "^2.2.6", "classnames": "^2.2.6",
"dom-helpers": "^5.1.2", "dom-helpers": "^5.1.2",
......
This diff is collapsed.
...@@ -68,47 +68,47 @@ describe('<Carousel>', () => { ...@@ -68,47 +68,47 @@ describe('<Carousel>', () => {
.simulate('click'); .simulate('click');
}); });
it('should call onSelect with previous direction and event', done => { ['onSlide', 'onSlid'].forEach(eventName => {
function onSelect(index, direction, event) { it(`should call ${eventName} with previous index and direction`, done => {
expect(index).to.equal(0); function onEvent(index, direction) {
expect(direction).to.equal('prev'); expect(index).to.equal(0);
expect(event).to.exist; expect(direction).to.equal('right');
done(); done();
} }
const wrapper = mount( const wrapper = mount(
<Carousel activeIndex={1} onSelect={onSelect}> <Carousel defaultActiveIndex={1} {...{ [eventName]: onEvent }}>
{items} {items}
</Carousel>, </Carousel>,
); );
wrapper wrapper
.find('.carousel-indicators li') .find('.carousel-indicators li')
.first() .first()
.simulate('click'); .simulate('click');
}); });
it('should call onSelect with next direction and event', done => { it(`should call ${eventName} with next index and direction`, done => {
function onSelect(index, direction, event) { function onEvent(index, direction) {
const lastPossibleIndex = items.length - 1; const lastPossibleIndex = items.length - 1;
expect(index).to.equal(lastPossibleIndex); expect(index).to.equal(lastPossibleIndex);
expect(direction).to.equal('next'); expect(direction).to.equal('left');
expect(event).to.exist;
done(); done();
} }
const wrapper = mount( const wrapper = mount(
<Carousel activeIndex={1} onSelect={onSelect}> <Carousel defaultActiveIndex={1} {...{ [eventName]: onEvent }}>
{items} {items}
</Carousel>, </Carousel>,
); );
wrapper wrapper
.find('.carousel-indicators li') .find('.carousel-indicators li')
.last() .last()
.simulate('click'); .simulate('click');
});
}); });
describe('Buttons and labels with and without wrapping', () => { describe('Buttons and labels with and without wrapping', () => {
...@@ -272,7 +272,7 @@ describe('<Carousel>', () => { ...@@ -272,7 +272,7 @@ describe('<Carousel>', () => {
{items} {items}
</Carousel>, </Carousel>,
); );
clock.tick(interval * 2); clock.tick(interval * 1.5);
expect(onSelectSpy).to.have.been.calledOnce; expect(onSelectSpy).to.have.been.calledOnce;
}); });
...@@ -285,11 +285,11 @@ describe('<Carousel>', () => { ...@@ -285,11 +285,11 @@ describe('<Carousel>', () => {
</Carousel>, </Carousel>,
); );
wrapper.simulate('mouseOver'); wrapper.simulate('mouseOver');
clock.tick(interval * 2); clock.tick(interval * 1.5);
sinon.assert.notCalled(onSelectSpy); sinon.assert.notCalled(onSelectSpy);
wrapper.simulate('mouseOut'); wrapper.simulate('mouseOut');
clock.tick(interval * 2); clock.tick(interval * 1.5);
sinon.assert.calledOnce(onSelectSpy); sinon.assert.calledOnce(onSelectSpy);
}); });
...@@ -297,17 +297,13 @@ describe('<Carousel>', () => { ...@@ -297,17 +297,13 @@ describe('<Carousel>', () => {
const onSelectSpy = sinon.spy(); const onSelectSpy = sinon.spy();
const interval = 500; const interval = 500;
const wrapper = mount( const wrapper = mount(
<Carousel <Carousel interval={interval} onSelect={onSelectSpy} pause={false}>
interval={interval}
onSelect={onSelectSpy}
pauseOnHover={false}
>
{items} {items}
</Carousel>, </Carousel>,
); );
wrapper.simulate('mouseOver'); wrapper.simulate('mouseOver');
clock.tick(interval * 2); clock.tick(interval * 1.5);
expect(onSelectSpy).to.have.been.calledOnce; expect(onSelectSpy).to.have.been.calledOnce;
}); });
...@@ -320,7 +316,7 @@ describe('<Carousel>', () => { ...@@ -320,7 +316,7 @@ describe('<Carousel>', () => {
</Carousel>, </Carousel>,
); );
wrapper.unmount(); wrapper.unmount();
clock.tick(interval * 2); clock.tick(interval * 1.5);
expect(onSelectSpy).not.to.have.been.called; expect(onSelectSpy).not.to.have.been.called;
}); });
}); });
...@@ -340,7 +336,7 @@ describe('<Carousel>', () => { ...@@ -340,7 +336,7 @@ describe('<Carousel>', () => {
const onSelectSpy = sinon.spy(); const onSelectSpy = sinon.spy();
const wrapper = mount( const wrapper = mount(
<Carousel activeIndex={0} interval={0} onSelect={onSelectSpy}> <Carousel activeIndex={0} onSelect={onSelectSpy}>
{items} {items}
</Carousel>, </Carousel>,
); );
...@@ -349,19 +345,15 @@ describe('<Carousel>', () => { ...@@ -349,19 +345,15 @@ describe('<Carousel>', () => {
key: 'ArrowLeft', key: 'ArrowLeft',
}); });
clock.tick(50); clock.tick(50);
sinon.assert.calledOnce(onSelectSpy);
sinon.assert.calledWith(onSelectSpy, items.length - 1); expect(onSelectSpy).to.have.been.calledOnceWith(items.length - 1);
}); });
it('should wrap from first to last', () => { it('should wrap from first to last', () => {
const onSelectSpy = sinon.spy(); const onSelectSpy = sinon.spy();
const wrapper = mount( const wrapper = mount(
<Carousel <Carousel activeIndex={items.length - 1} onSelect={onSelectSpy}>
activeIndex={items.length - 1}
interval={0}
onSelect={onSelectSpy}
>
{items} {items}
</Carousel>, </Carousel>,
); );
...@@ -370,8 +362,8 @@ describe('<Carousel>', () => { ...@@ -370,8 +362,8 @@ describe('<Carousel>', () => {
key: 'ArrowRight', key: 'ArrowRight',
}); });
clock.tick(50); clock.tick(50);
sinon.assert.calledOnce(onSelectSpy);
sinon.assert.calledWith(onSelectSpy, 0); expect(onSelectSpy).to.have.been.calledOnceWith(0);
}); });
[ [
...@@ -396,7 +388,6 @@ describe('<Carousel>', () => { ...@@ -396,7 +388,6 @@ describe('<Carousel>', () => {
<Carousel <Carousel
activeIndex={activeIndex} activeIndex={activeIndex}
wrap={false} wrap={false}
interval={0}
onSelect={onSelectSpy} onSelect={onSelectSpy}
> >
{items} {items}
...@@ -415,6 +406,7 @@ describe('<Carousel>', () => { ...@@ -415,6 +406,7 @@ describe('<Carousel>', () => {
describe('keyboard events', () => { describe('keyboard events', () => {
let clock; let clock;
beforeEach(() => { beforeEach(() => {
clock = sinon.useFakeTimers(); clock = sinon.useFakeTimers();
}); });
...@@ -426,7 +418,7 @@ describe('<Carousel>', () => { ...@@ -426,7 +418,7 @@ describe('<Carousel>', () => {
it('should go back for the keyboard event ArrowLeft', () => { it('should go back for the keyboard event ArrowLeft', () => {
const onSelectSpy = sinon.spy(); const onSelectSpy = sinon.spy();
const wrapper = mount( const wrapper = mount(
<Carousel activeIndex={1} interval={0} onSelect={onSelectSpy}> <Carousel activeIndex={1} onSelect={onSelectSpy}>
{items} {items}
</Carousel>, </Carousel>,
); );
...@@ -442,7 +434,7 @@ describe('<Carousel>', () => { ...@@ -442,7 +434,7 @@ describe('<Carousel>', () => {
it('should go forward for the keyboard event ArrowRight', () => { it('should go forward for the keyboard event ArrowRight', () => {
const onSelectSpy = sinon.spy(); const onSelectSpy = sinon.spy();
const wrapper = mount( const wrapper = mount(
<Carousel activeIndex={1} interval={0} onSelect={onSelectSpy}> <Carousel activeIndex={1} onSelect={onSelectSpy}>
{items} {items}
</Carousel>, </Carousel>,
); );
...@@ -458,12 +450,7 @@ describe('<Carousel>', () => { ...@@ -458,12 +450,7 @@ describe('<Carousel>', () => {
it('should ignore keyEvents when the keyboard is disabled', () => { it('should ignore keyEvents when the keyboard is disabled', () => {
const onSelectSpy = sinon.spy(); const onSelectSpy = sinon.spy();
const wrapper = mount( const wrapper = mount(
<Carousel <Carousel activeIndex={1} onSelect={onSelectSpy} keyboard={false}>
activeIndex={1}
interval={0}
onSelect={onSelectSpy}
keyboard={false}
>
{items} {items}
</Carousel>, </Carousel>,
); );
...@@ -475,11 +462,11 @@ describe('<Carousel>', () => { ...@@ -475,11 +462,11 @@ describe('<Carousel>', () => {
sinon.assert.notCalled(onSelectSpy); sinon.assert.notCalled(onSelectSpy);
}); });
['ArrowUp', 'ArrowRightLeft', 'Onwards'].forEach(({ key }) => { ['ArrowUp', 'ArrowRightLeft', 'Onwards'].forEach(key => {
it('should do nothing for non left or right keys', () => { it('should do nothing for non left or right keys', () => {
const onSelectSpy = sinon.spy(); const onSelectSpy = sinon.spy();
const wrapper = mount( const wrapper = mount(
<Carousel activeIndex={1} interval={0} onSelect={onSelectSpy}> <Carousel activeIndex={1} onSelect={onSelectSpy}>
{items} {items}
</Carousel>, </Carousel>,
); );
...@@ -509,40 +496,39 @@ describe('<Carousel>', () => { ...@@ -509,40 +496,39 @@ describe('<Carousel>', () => {
afterEach(() => { afterEach(() => {
clock.restore(); clock.restore();
clock.tick(150);
}); });
it('should swipe right', () => { it('should swipe right', () => {
wrapper.simulate('touchStart', { changedTouches: [{ screenX: 50 }] }); wrapper.simulate('touchStart', { touches: [{ clientX: 50 }] });
wrapper.simulate('touchEnd', { changedTouches: [{ screenX: 0 }] }); wrapper.simulate('touchMove', { touches: [{ clientX: 0 }] });
wrapper.simulate('touchEnd');
clock.tick(50); clock.tick(50);
expect(onSelectSpy).to.have.been.calledOnce;
expect(onSelectSpy.getCall(0).args[0]).to.equal(2); expect(onSelectSpy).to.have.been.calledOnceWith(2);
}); });
it('should swipe left', () => { it('should swipe left', () => {
wrapper.simulate('touchStart', { changedTouches: [{ screenX: 0 }] }); wrapper.simulate('touchStart', { touches: [{ clientX: 0 }] });
wrapper.simulate('touchEnd', { changedTouches: [{ screenX: 50 }] }); wrapper.simulate('touchMove', { touches: [{ clientX: 50 }] });
wrapper.simulate('touchEnd');
clock.tick(50); clock.tick(50);
expect(onSelectSpy).to.have.been.calledOnce;
expect(onSelectSpy.getCall(0).args[0]).to.equal(0); expect(onSelectSpy).to.have.been.calledOnceWith(0);
}); });
it('should not swipe if swipe detected is under the swipe threshold', () => { it('should not swipe if swipe detected is under the swipe threshold', () => {
const SWIPE_THRESHOLD = 40; wrapper.simulate('touchStart', { touches: [{ clientX: 0 }] });
wrapper.simulate('touchStart', { changedTouches: [{ screenX: 0 }] }); wrapper.simulate('touchMove', { touches: [{ clientX: 35 }] });
wrapper.simulate('touchEnd', { wrapper.simulate('touchEnd');
changedTouches: [{ screenX: SWIPE_THRESHOLD - 5 }],
});
clock.tick(50); clock.tick(50);
expect(onSelectSpy).to.not.have.been.called; expect(onSelectSpy).to.not.have.been.called;
}); });
it('should do nothing with disabled touch right', () => { it('should do nothing with disabled touch right', () => {
const noTochWrapper = mount( const noTouchWrapper = mount(
<Carousel <Carousel
activeIndex={1} activeIndex={1}
interval={null} interval={null}
...@@ -552,12 +538,13 @@ describe('<Carousel>', () => { ...@@ -552,12 +538,13 @@ describe('<Carousel>', () => {
{items} {items}
</Carousel>, </Carousel>,
); );
noTochWrapper.simulate('touchStart', { noTouchWrapper.simulate('touchStart', {
changedTouches: [{ screenX: 50 }], touches: [{ clientX: 50 }],
}); });
noTochWrapper.simulate('touchEnd', { noTouchWrapper.simulate('touchMove', {
changedTouches: [{ screenX: 0 }], touches: [{ clientX: 0 }],
}); });
noTouchWrapper.simulate('touchEnd');
clock.tick(50); clock.tick(50);
expect(onSelectSpy).to.not.have.been.called; expect(onSelectSpy).to.not.have.been.called;
......
...@@ -9,20 +9,22 @@ export interface CarouselProps { ...@@ -9,20 +9,22 @@ export interface CarouselProps {
bsPrefix?: string; bsPrefix?: string;
slide?: boolean; slide?: boolean;
fade?: boolean; fade?: boolean;
wrap?: boolean; controls?: boolean;
indicators?: boolean; indicators?: boolean;
activeIndex?: number;
onSelect?: (eventKey: number, event: object | null) => void;
defaultActiveIndex?: number;
onSlide?: (eventKey: number, direction: 'left' | 'right') => void;
onSlid?: (eventKey: number, direction: 'left' | 'right') => void;
interval?: number | null; interval?: number | null;
controls?: boolean;
pauseOnHover?: boolean;
keyboard?: boolean; keyboard?: boolean;
onSelect?: (eventKey: any, direction: 'prev' | 'next', event: object) => void; pause?: 'hover' | false;
onSlideEnd?: () => void; wrap?: boolean;
activeIndex?: number; touch?: boolean;
prevIcon?: React.ReactNode; prevIcon?: React.ReactNode;
prevLabel?: string; prevLabel?: React.ReactNode;
nextIcon?: React.ReactNode; nextIcon?: React.ReactNode;
nextLabel?: string; nextLabel?: React.ReactNode;
touch?: boolean;
} }
declare class Carousel< declare class Carousel<
......
...@@ -951,7 +951,7 @@ ...@@ -951,7 +951,7 @@
resolved "https://registry.yarnpkg.com/@restart/context/-/context-2.1.4.tgz#a99d87c299a34c28bd85bb489cb07bfd23149c02" resolved "https://registry.yarnpkg.com/@restart/context/-/context-2.1.4.tgz#a99d87c299a34c28bd85bb489cb07bfd23149c02"
integrity sha512-INJYZQJP7g+IoDUh/475NlGiTeMfwTXUEr3tmRneckHIxNolGOW9CTq83S8cxq0CgJwwcMzMJFchxvlwe7Rk8Q== integrity sha512-INJYZQJP7g+IoDUh/475NlGiTeMfwTXUEr3tmRneckHIxNolGOW9CTq83S8cxq0CgJwwcMzMJFchxvlwe7Rk8Q==
"@restart/hooks@^0.3.11", "@restart/hooks@^0.3.12": "@restart/hooks@^0.3.12", "@restart/hooks@^0.3.21":
version "0.3.21" version "0.3.21"
resolved "https://registry.yarnpkg.com/@restart/hooks/-/hooks-0.3.21.tgz#5264d12019ffb844dc1fc44d55517ded7b580ee2" resolved "https://registry.yarnpkg.com/@restart/hooks/-/hooks-0.3.21.tgz#5264d12019ffb844dc1fc44d55517ded7b580ee2"
integrity sha512-Wcu3CFJV+iiqPEIoPVx3/CYnZBRgPeRABo6bLJByRH9ptJXyObn7WYPG7Rv0cg3+55bqcBbG0xEfovzwE2PNXg== integrity sha512-Wcu3CFJV+iiqPEIoPVx3/CYnZBRgPeRABo6bLJByRH9ptJXyObn7WYPG7Rv0cg3+55bqcBbG0xEfovzwE2PNXg==
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment