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

[XGrid] Second iteration on resizing logic #436

Merged
merged 5 commits into from
Oct 19, 2020

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Oct 11, 2020

jump

  • Fix scrolling on the last column

scroll

  • Fix alternatives button clicks, e.g. if you click on the previous page button while your mouse is over the dragging area, it fails.

  • Fix interaction area, the whole separator should be interactive

zone

  • Fix keep resizing even after releasing the mouse

keep resizing

@oliviertassinari oliviertassinari changed the title [XGrid] Fix resize in strict mode [XGrid] Second iteration on the logic Oct 11, 2020
@oliviertassinari oliviertassinari force-pushed the fix-resize-strict-mode branch 2 times, most recently from e4811ac to 11e65c2 Compare October 11, 2020 19:20
@oliviertassinari oliviertassinari marked this pull request as ready for review October 11, 2020 19:21
@oliviertassinari oliviertassinari changed the title [XGrid] Second iteration on the logic [XGrid] Second iteration on resizing logic Oct 11, 2020
@oliviertassinari
Copy link
Member Author

@oliviertassinari
Copy link
Member Author

A demonstration that the fix for #177 is working: https://codesandbox.io/s/modest-herschel-4vkmo?file=/package.json

docs/next.config.js Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari force-pushed the fix-resize-strict-mode branch 3 times, most recently from d8731e3 to 5ccf943 Compare October 13, 2020 20:10
@oliviertassinari oliviertassinari force-pushed the fix-resize-strict-mode branch 4 times, most recently from aff8d59 to b5a20e8 Compare October 15, 2020 23:57
}),
};
const width = column.width!;
const dragConfig = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usememo? avoid to recreate the functions and rerender the subtree...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind that if you review with:

Capture d’écran 2020-10-19 à 15 42 29

You will see that this line is not linked to the resizing change.

Copy link
Member Author

@oliviertassinari oliviertassinari Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the value is spread, so no performance issues here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the value is spread but when you do (event)=> something(event) it creates a new function on each render and so shallow equal will never work and the sub tree will always rerender

Copy link
Member Author

@oliviertassinari oliviertassinari Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's applied on a div, so using a memo won't help performance. It won't prune a large react rendering tree, the only case where it helps with performance.

@oliviertassinari oliviertassinari force-pushed the fix-resize-strict-mode branch 2 times, most recently from a1b1e1e to f0c5ffc Compare October 19, 2020 15:51
@oliviertassinari oliviertassinari force-pushed the fix-resize-strict-mode branch 2 times, most recently from 58ebf1e to f477235 Compare October 19, 2020 16:03
@oliviertassinari oliviertassinari merged commit c8245c0 into mui:master Oct 19, 2020
@oliviertassinari oliviertassinari deleted the fix-resize-strict-mode branch October 19, 2020 19:18
dtassone pushed a commit to dtassone/material-ui-x that referenced this pull request Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
3 participants