Skip to content

Commit

Permalink
fix(react-grid): fix column resizing after its reordering (T1096930) (#…
Browse files Browse the repository at this point in the history 
…3557)
  • Loading branch information
@Krijovnick
Krijovnick committed Jul 4, 2022
1 parent 9e420bd commit ae205dd
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 27 deletions.
2 changes: 1 addition & 1 deletion packages/dx-react-chart/src/utils/updatable-sizer.tsx
View file
Expand Up @@ -7,7 +7,7 @@ export class UpdatableSizer extends React.PureComponent<SizerProps> {
ref = React.createRef<Sizer>();

componentDidUpdate() {
this.ref.current!.setupListeners();
this.props.onSizeChange(this.ref.current!.getSize());
}

render() {
Expand Down
105 changes: 99 additions & 6 deletions packages/dx-react-core/src/sizer.test.tsx
View file
Expand Up @@ -6,10 +6,10 @@ describe('Sizer', () => {
const divProto = (document.createElement('div') as HTMLDivElement).constructor.prototype;
let addEventListener: any;
let removeEventListener: any;
const Container = ({ forwardedRef }) => (
<div ref={forwardedRef} className="container" />
const Container = ({ forwardedRef, ...restProps }) => (
<div ref={forwardedRef} className="container" {...restProps} />
);

const onSizeChange = jest.fn();
beforeAll(() => {
addEventListener = divProto.addEventListener;
removeEventListener = divProto.removeEventListener;
Expand All @@ -25,12 +25,36 @@ describe('Sizer', () => {
afterEach(() => {
divProto.addEventListener.mockClear();
divProto.removeEventListener.mockClear();
onSizeChange.mockClear();
});

it('should create component with childrens', () => {
const tree = mount(
<Sizer
onSizeChange={onSizeChange}
containerComponent={Container}
style={{ key: 'test style' }}
/>,
);

const root = tree.find('.container');
expect(root.props()).toEqual({
className: 'container',
style: {
key: 'test style',
position: 'relative',
},
});
expect(root.getDOMNode().childNodes.length).toBe(1);
expect(root.getDOMNode().childNodes[0].childNodes.length).toBe(2);
expect(root.getDOMNode().childNodes[0].childNodes[0].childNodes.length).toBe(1);
expect(root.getDOMNode().childNodes[0].childNodes[1].childNodes.length).toBe(1);
});

it('should add listeners on mount', () => {
const tree = mount(
<Sizer
onSizeChange={() => void 0}
onSizeChange={onSizeChange}
containerComponent={Container}
/>,
);
Expand All @@ -52,7 +76,7 @@ describe('Sizer', () => {
it('should remove listeners on unmount', () => {
const tree = mount(
<Sizer
onSizeChange={() => void 0}
onSizeChange={onSizeChange}
containerComponent={Container}
/>,
);
Expand All @@ -73,7 +97,7 @@ describe('Sizer', () => {
it('should set a 2px scroll offset to notifiers', () => {
const tree = mount(
<Sizer
onSizeChange={() => void 0}
onSizeChange={onSizeChange}
containerComponent={Container}
/>,
);
Expand All @@ -88,4 +112,73 @@ describe('Sizer', () => {
expect(expandNotifier.style.width).toBe('2px');
expect(expandNotifier.style.height).toBe('2px');
});

it('should call onSizeChange on mount', () => {
const tree = mount(
<Sizer
onSizeChange={onSizeChange}
containerComponent={Container}
/>,
);
const instance = tree.instance() as any;
instance.getSize = jest.fn().mockReturnValue({ width: 20, height: 10 });
instance.componentDidMount();

expect(onSizeChange).toBeCalledWith({ width: 20, height: 10 });
});

it('should update scroll offsets to rootNode', () => {
const tree = mount(
<Sizer
onSizeChange={onSizeChange}
containerComponent={Container}
scrollTop={35}
scrollLeft={45}
/>,
);
const instance = tree.instance() as any;
instance.componentDidUpdate();

expect(instance.rootRef.current.scrollTop).toBe(35);
expect(instance.rootRef.current.scrollLeft).toBe(45);
});

// T1096930
it('should update scroll offsets to notifiers', () => {
const resetOffsets = () => {
// after column reordering scroll offsets are resets
(root.firstChild!.childNodes[0] as any).scrollTop = 0;
(root.firstChild!.childNodes[0] as any).scrollLeft = 0;
(root.firstChild!.childNodes[1] as any).scrollTop = 0;
(root.firstChild!.childNodes[1] as any).scrollLeft = 0;
};
const tree = mount(
<Sizer
onSizeChange={onSizeChange}
containerComponent={Container}
/>,
);
const instance = tree.instance() as any;

const root = tree.find('.container').getDOMNode();
resetOffsets();
instance.getSize = jest.fn().mockReturnValue({ width: 20, height: 10 });
instance.componentDidUpdate();

expect((root.firstChild!.childNodes[0] as any).scrollTop).toBe(2);
expect((root.firstChild!.childNodes[0] as any).scrollLeft).toBe(2);

expect((root.firstChild!.childNodes[1] as any).scrollTop).toBe(10);
expect((root.firstChild!.childNodes[1] as any).scrollLeft).toBe(20);

resetOffsets();
instance.getSize = jest.fn().mockReturnValue({ width: 30, height: 20 });
instance.componentDidUpdate();

expect((root.firstChild!.childNodes[0] as any).scrollTop).toBe(2);
expect((root.firstChild!.childNodes[0] as any).scrollLeft).toBe(2);

expect((root.firstChild!.childNodes[1] as any).scrollTop).toBe(20);
expect((root.firstChild!.childNodes[1] as any).scrollLeft).toBe(30);
});
});
64 changes: 44 additions & 20 deletions packages/dx-react-core/src/sizer.tsx
View file
Expand Up @@ -2,8 +2,10 @@

import * as React from 'react';
import { SizerProps, Size } from './types';
import { shallowEqual } from '@devexpress/dx-core';

const styles = {
const SCROLL_OFFSET = 2;
const styles: Record<string, React.CSSProperties> = {
root: {
position: 'relative',
},
Expand Down Expand Up @@ -48,14 +50,13 @@ const styles = {
};

/** @internal */
export class Sizer extends React.PureComponent<SizerProps> {
export class Sizer extends React.Component<SizerProps> {
static defaultProps = {
containerComponent: 'div',
};

rootRef: React.RefObject<unknown>;
// Though there properties cannot be assigned in constructor
// they will be assigned when component is mount.
rootRef: React.RefObject<HTMLElement>;

rootNode!: HTMLElement;
triggersRoot!: HTMLDivElement;
expandTrigger!: HTMLDivElement;
Expand All @@ -67,25 +68,43 @@ export class Sizer extends React.PureComponent<SizerProps> {
super(props);

this.setupListeners = this.setupListeners.bind(this);
this.updateScrolling = this.updateScrolling.bind(this);
this.rootRef = React.createRef();
}

componentDidMount() {
this.rootNode = this.rootRef.current!;
this.createListeners();

this.expandTrigger.addEventListener('scroll', this.setupListeners);
this.contractTrigger.addEventListener('scroll', this.setupListeners);
this.setupListeners();
}

shouldComponentUpdate(prevProps) {
if (prevProps.scrollTop !== this.props.scrollTop ||
prevProps.scrollLeft !== this.props.scrollLeft ||
(prevProps.style && this.props.style &&
!shallowEqual(prevProps.style, this.props.style)) ||
(prevProps.style && !this.props.style) ||
prevProps.children !== this.props.children) {
return true;
}
return false;
}

componentDidUpdate() {
// We can scroll the VirtualTable manually only by changing
// containter's (rootNode) scrollTop property.
// Viewport changes its own properties automatically.
const { scrollTop, scrollLeft } = this.props;
if (scrollTop! > -1) {
this.rootNode.scrollTop = scrollTop!;
if (scrollTop !== undefined && scrollTop > -1) {
this.rootNode.scrollTop = scrollTop;
}
if (scrollLeft! > -1) {
this.rootNode.scrollLeft = scrollLeft!;
if (scrollLeft !== undefined && scrollLeft > -1) {
this.rootNode.scrollLeft = scrollLeft;
}
this.updateScrolling(this.getSize());
}

// There is no need to remove listeners as divs are removed from DOM when component is unmount.
Expand All @@ -96,31 +115,28 @@ export class Sizer extends React.PureComponent<SizerProps> {
this.contractTrigger.removeEventListener('scroll', this.setupListeners);
}

getSize = (): Size => ({ height: this.rootNode.clientHeight, width: this.rootNode.clientWidth });
getSize = (): Size => ({
height: this.rootNode.clientHeight,
width: this.rootNode.clientWidth,
})

setupListeners() {
const size = this.getSize();
const { width, height } = size;

this.contractTrigger.scrollTop = height;
this.contractTrigger.scrollLeft = width;
this.expandNotifier.style.width = `${width + SCROLL_OFFSET}px`;
this.expandNotifier.style.height = `${height + SCROLL_OFFSET}px`;

const scrollOffset = 2;
this.expandNotifier.style.width = `${width + scrollOffset}px`;
this.expandNotifier.style.height = `${height + scrollOffset}px`;
this.expandTrigger.scrollTop = scrollOffset;
this.expandTrigger.scrollLeft = scrollOffset;
this.updateScrolling(size);

const { onSizeChange } = this.props;
onSizeChange(size);
}

createListeners() {
this.rootNode = this.rootRef.current as HTMLElement;

this.triggersRoot = document.createElement('div');
Object.assign(this.triggersRoot.style, styles.triggersRoot);
this.rootNode.appendChild(this.triggersRoot);
this.rootNode!.appendChild(this.triggersRoot);

this.expandTrigger = document.createElement('div');
Object.assign(this.expandTrigger.style, styles.expandTrigger);
Expand All @@ -140,6 +156,14 @@ export class Sizer extends React.PureComponent<SizerProps> {
this.contractTrigger.appendChild(this.contractNotifier);
}

updateScrolling(size) {
const { width, height } = size;
this.contractTrigger.scrollTop = height;
this.contractTrigger.scrollLeft = width;
this.expandTrigger.scrollTop = SCROLL_OFFSET;
this.expandTrigger.scrollLeft = SCROLL_OFFSET;
}

render() {
const {
onSizeChange,
Expand Down

0 comments on commit ae205dd

Please sign in to comment.