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.

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

fix: masonry scroll handler should rely on event.currentTarget issue#1414 #1420

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 33 additions & 1 deletion source/Masonry/Masonry.jest.js
Expand Up @@ -97,7 +97,10 @@ function getMarkup(props = {}) {
function simulateScroll(masonry, scrollTop = 0) {
const target = {scrollTop};
masonry._scrollingContainer = target; // HACK to work around _onScroll target check
Simulate.scroll(findDOMNode(masonry), {target});

const masonryNode = findDOMNode(masonry);
masonryNode.scrollTop = scrollTop;
Simulate.scroll(masonryNode);
}

describe('Masonry', () => {
Expand Down Expand Up @@ -298,6 +301,35 @@ describe('Masonry', () => {
expect(node.style.right).toMatch(/px/);
});
});

it('should consider scroll only of the container element and not of any ancestor element', () => {
const cellMeasurerCache = createCellMeasurerCache();
const renderScrollableCell = index => (
<div
style={{height: '50px', overflow: 'visible'}}
id={`scrollable-cell-${index}`}>
<div style={{height: '500px'}}>{index}</div>
</div>
);
const cellRenderer = createCellRenderer(
cellMeasurerCache,
renderScrollableCell,
);

const rendered = findDOMNode(
render(
getMarkup({
overscanByPixels: 0,
cellMeasurerCache,
cellRenderer,
}),
),
);
assertVisibleCells(rendered, '0,1,2,3,4,5');
const cellEl = rendered.querySelector('#scrollable-cell-1');
Simulate.scroll(cellEl, {target: {scrollTop: 100}});
assertVisibleCells(rendered, '0,1,2,3,4,5');
});
});

describe('recomputeCellPositions', () => {
Expand Down
2 changes: 1 addition & 1 deletion source/Masonry/Masonry.js
Expand Up @@ -410,7 +410,7 @@ class Masonry extends React.PureComponent<Props, State> {
_onScroll = event => {
const {height} = this.props;

const eventScrollTop = event.target.scrollTop;
const eventScrollTop = event.currentTarget.scrollTop;

// When this component is shrunk drastically, React dispatches a series of back-to-back scroll events,
// Gradually converging on a scrollTop that is within the bounds of the new, smaller height.
Expand Down