Skip to content

Commit

Permalink
Fix calculation of slides width and height, fixes #495 (#525)
Browse files Browse the repository at this point in the history
  • Loading branch information
karolisgrinkevicius-home24 authored and Sarah Meyer committed Apr 10, 2019
1 parent 808b047 commit ccbffd1
Show file tree
Hide file tree
Showing 8 changed files with 3,400 additions and 3,625 deletions.
3 changes: 3 additions & 0 deletions .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
["@babel/preset-env", { "modules": false }],
"@babel/preset-react"
]
},
"test": {
"plugins": ["@babel/plugin-transform-runtime"]
}
}
}
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
node_modules
lib
es
dist
dist
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
language: node_js

node_js:
- '6'
- '8'
- '10'

sudo: false

Expand Down
2 changes: 1 addition & 1 deletion jest.unit.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module.exports = {
testEnvironment: 'enzyme',
setupTestFrameworkScriptFile: './node_modules/jest-enzyme/lib/index.js',
setupFilesAfterEnv: ['./node_modules/jest-enzyme/lib/index.js'],
roots: ['<rootDir>/test/specs/']
};
31 changes: 18 additions & 13 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,24 @@
"react-move": "^5.0.0"
},
"devDependencies": {
"@babel/cli": "^7.0.0-beta.42",
"@babel/core": "^7.0.0-beta.42",
"@babel/plugin-proposal-object-rest-spread": "^7.0.0",
"@babel/plugin-transform-object-assign": "^7.0.0-beta.42",
"@babel/polyfill": "^7.0.0-beta.42",
"@babel/preset-env": "^7.0.0-beta.42",
"@babel/preset-react": "^7.0.0-beta.42",
"@babel/cli": "^7.0.0",
"@babel/core": "^7.0.0",
"@babel/plugin-proposal-object-rest-spread": "^7.4.3",
"@babel/plugin-transform-object-assign": "^7.2.0",
"@babel/plugin-transform-runtime": "^7.4.3",
"@babel/polyfill": "^7.0.0",
"@babel/preset-env": "^7.0.0",
"@babel/preset-react": "^7.0.0",
"babel-core": "7.0.0-bridge.0",
"babel-eslint": "^8.2.2",
"babel-jest": "^22.4.1",
"babel-jest": "^24.7.1",
"babel-loader": "8.0.0-beta.2",
"builder": "^4.0.0",
"chai": "^3.3.0",
"css-loader": "^0.28.10",
"del": "^2.0.2",
"enzyme": "^3.3.0",
"enzyme-adapter-react-16": "^1.1.1",
"enzyme-adapter-react-16": "^1.11.2",
"eslint": "^4.8.0",
"eslint-config-formidable": "^3.0.0",
"eslint-config-jest-enzyme": "^6.0.0",
Expand All @@ -41,10 +42,11 @@
"eslint-plugin-import": "^2.9.0",
"eslint-plugin-prettier": "^2.6.0",
"eslint-plugin-react": "^7.4.0",
"jest": "^22.4.2",
"jest-environment-enzyme": "^6.0.0",
"jest-enzyme": "^6.0.0",
"jest": "^24.7.1",
"jest-environment-enzyme": "^7.0.2",
"jest-enzyme": "^7.0.2",
"jest-puppeteer-preset": "^2.0.1",
"jsdom": "^14.0.0",
"prettier": "1.15.3",
"puppeteer": "^1.1.1",
"react": "^16.0.0",
Expand All @@ -54,7 +56,7 @@
"sinon": "^4.4.2",
"style-loader": "^0.20.2",
"tslint": "^5.12.0",
"typescript": "^3.2.2",
"typescript": "^3.4.1",
"url-loader": "^0.6.2",
"webpack": "4.19.1",
"webpack-cli": "3.1.1",
Expand All @@ -64,6 +66,9 @@
"react": "^0.14.9 || ^15.3.0 || ^16.0.0",
"react-dom": "^0.14.9 || ^15.3.0 || ^16.0.0"
},
"resolutions": {
"jsdom": "^14.0.0"
},
"scripts": {
"build": "builder concurrent --buffer build-lib build-es",
"build-babel": "babel src",
Expand Down
36 changes: 36 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export default class Carousel extends React.Component {
{ funcName: 'renderBottomCenterControls', key: 'BottomCenter' },
{ funcName: 'renderBottomRightControls', key: 'BottomRight' }
];
this.childNodesMutationObs = null;

this.state = {
currentSlide: this.props.slideIndex,
Expand Down Expand Up @@ -88,6 +89,9 @@ export default class Carousel extends React.Component {
this.setSlideHeightAndWidth = this.setSlideHeightAndWidth.bind(this);
this.startAutoplay = this.startAutoplay.bind(this);
this.stopAutoplay = this.stopAutoplay.bind(this);
this.establishChildNodesMutationObserver = this.establishChildNodesMutationObserver.bind(
this
);
}

componentDidMount() {
Expand All @@ -96,6 +100,7 @@ export default class Carousel extends React.Component {
this.setLeft();
this.setDimensions();
this.bindEvents();
this.establishChildNodesMutationObserver();
if (this.props.autoplay) {
this.startAutoplay();
}
Expand Down Expand Up @@ -165,11 +170,42 @@ export default class Carousel extends React.Component {

componentWillUnmount() {
this.unbindEvents();
this.disconnectChildNodesMutationObserver();
this.stopAutoplay();
// see https://github.com/facebook/react/issues/3417#issuecomment-121649937
this.mounted = false;
}

establishChildNodesMutationObserver() {
const childNodes = this.getChildNodes();
if (childNodes.length && 'MutationObserver' in window) {
this.childNodesMutationObs = new MutationObserver(mutations => {
mutations.forEach(() => {
this.setSlideHeightAndWidth();
});
});

const observeChildNodeMutation = node => {
this.childNodesMutationObs.observe(node, {
attributeFilter: ['style'],
attributeOldValue: false,
characterData: false,
characterDataOldValue: false,
childList: false,
subtree: false
});
};

childNodes.forEach(observeChildNodeMutation);
}
}

disconnectChildNodesMutationObserver() {
if (this.childNodesMutationObs instanceof MutationObserver) {
this.childNodesMutationObs.disconnect();
}
}

getTouchEvents() {
if (this.props.swiping === false) {
return {
Expand Down
115 changes: 95 additions & 20 deletions test/specs/carousel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,24 @@ describe('<Carousel />', () => {
});

it('should set slideHeight to max value by default', () => {
const firstSlideNode = document.createElement('div');
const secondSlideNode = document.createElement('div');
const thirdSlideNode = document.createElement('div');
Object.defineProperties(firstSlideNode, {
offsetHeight: { value: 100 },
style: {}
});
Object.defineProperties(secondSlideNode, {
offsetHeight: { value: 200 },
style: {}
});
Object.defineProperties(thirdSlideNode, {
offsetHeight: { value: 300 },
style: {}
});
jest
.spyOn(Carousel.prototype, 'getChildNodes')
.mockReturnValue([
{ offsetHeight: 100, style: {} },
{ offsetHeight: 200, style: {} },
{ offsetHeight: 300, style: {} }
]);
.mockReturnValue([firstSlideNode, secondSlideNode, thirdSlideNode]);
const wrapper = mount(
<Carousel>
<div style={{ height: '100px' }}>Slide 1</div>
Expand Down Expand Up @@ -287,13 +298,24 @@ describe('<Carousel />', () => {
});

it('should set slideHeight to max value when `heightMode` is `max`', () => {
const firstSlideNode = document.createElement('div');
const secondSlideNode = document.createElement('div');
const thirdSlideNode = document.createElement('div');
Object.defineProperties(firstSlideNode, {
offsetHeight: { value: 100 },
style: {}
});
Object.defineProperties(secondSlideNode, {
offsetHeight: { value: 200 },
style: {}
});
Object.defineProperties(thirdSlideNode, {
offsetHeight: { value: 300 },
style: {}
});
jest
.spyOn(Carousel.prototype, 'getChildNodes')
.mockReturnValue([
{ offsetHeight: 100, style: {} },
{ offsetHeight: 200, style: {} },
{ offsetHeight: 300, style: {} }
]);
.mockReturnValue([firstSlideNode, secondSlideNode, thirdSlideNode]);
const wrapper = mount(
<Carousel heightMode="max">
<div style={{ height: '100px' }}>Slide 1</div>
Expand All @@ -306,13 +328,24 @@ describe('<Carousel />', () => {
});

it("should set slideHeight to first slide's height when `heightMode` is `first`", () => {
const firstSlideNode = document.createElement('div');
const secondSlideNode = document.createElement('div');
const thirdSlideNode = document.createElement('div');
Object.defineProperties(firstSlideNode, {
offsetHeight: { value: 100 },
style: {}
});
Object.defineProperties(secondSlideNode, {
offsetHeight: { value: 200 },
style: {}
});
Object.defineProperties(thirdSlideNode, {
offsetHeight: { value: 300 },
style: {}
});
jest
.spyOn(Carousel.prototype, 'getChildNodes')
.mockReturnValue([
{ offsetHeight: 100, style: {} },
{ offsetHeight: 200, style: {} },
{ offsetHeight: 300, style: {} }
]);
.mockReturnValue([firstSlideNode, secondSlideNode, thirdSlideNode]);
const wrapper = mount(
<Carousel heightMode="first">
<div style={{ height: '100px' }}>Slide 1</div>
Expand All @@ -325,13 +358,24 @@ describe('<Carousel />', () => {
});

it('should set height to current slide height when `heightMode` is `current`', () => {
const firstSlideNode = document.createElement('div');
const secondSlideNode = document.createElement('div');
const thirdSlideNode = document.createElement('div');
Object.defineProperties(firstSlideNode, {
offsetHeight: { value: 100 },
style: {}
});
Object.defineProperties(secondSlideNode, {
offsetHeight: { value: 200 },
style: {}
});
Object.defineProperties(thirdSlideNode, {
offsetHeight: { value: 300 },
style: {}
});
jest
.spyOn(Carousel.prototype, 'getChildNodes')
.mockReturnValue([
{ offsetHeight: 100, style: {} },
{ offsetHeight: 200, style: {} },
{ offsetHeight: 300, style: {} }
]);
.mockReturnValue([firstSlideNode, secondSlideNode, thirdSlideNode]);
const wrapper = mount(
<Carousel heightMode="current" slideIndex={1}>
<div style={{ height: '100px' }}>Slide 1</div>
Expand Down Expand Up @@ -1001,5 +1045,36 @@ describe('<Carousel />', () => {
button.simulate('click');
expect(spy).toHaveBeenCalledWith(2);
});

it('should reset slide height and width once one of the child nodes is mutated by style attr', async () => {
const firstSlideNode = document.createElement('div');
const secondSlideNode = document.createElement('div');
jest
.spyOn(Carousel.prototype, 'getChildNodes')
.mockReturnValue([firstSlideNode, secondSlideNode]);
const wrapper = mount(
<Carousel>
<span style={{ height: '1px' }}>Slide 1</span>
<span style={{ height: '2px' }}>Slide 2</span>
</Carousel>
);
const instance = wrapper.instance();
const childNodesCount = instance.getChildNodes().length;
jest.spyOn(instance, 'setSlideHeightAndWidth');
expect(instance.setSlideHeightAndWidth).not.toBeCalled();
firstSlideNode.setAttribute('style', {});
// jest.advanceTimersByTime(1000);
await jest.setTimeout(0);
expect(instance.setSlideHeightAndWidth).toBeCalledTimes(childNodesCount);
instance.setSlideHeightAndWidth.mockClear();
firstSlideNode.setAttribute('style', { height: 1 });
await jest.setTimeout(0);
expect(instance.setSlideHeightAndWidth).toBeCalledTimes(childNodesCount);
instance.setSlideHeightAndWidth.mockClear();
firstSlideNode.setAttribute('data-fake-attr', '');
await jest.setTimeout(0);
expect(instance.setSlideHeightAndWidth).not.toBeCalled();
Carousel.prototype.getChildNodes.mockRestore();
});
});
});
Loading

0 comments on commit ccbffd1

Please sign in to comment.