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

Dynamically resize items #11

Closed
romanstetsyk opened this issue Jun 7, 2023 · 20 comments
Closed

Dynamically resize items #11

romanstetsyk opened this issue Jun 7, 2023 · 20 comments

Comments

@romanstetsyk
Copy link
Contributor

Is there any way to resize grid items dynamically like in this codesandbox that uses muuri-react

Here is a codesandbox with my attempt, but the items overlap on resize.

Any help would be appreciated, and thanks for creating this wrapper!

@vingeraycn
Copy link
Owner

Thank you very much for your like about ruuri. Referring to the two examples mentioned above, I make slight changes based on your example to see here if it helps you solve this issue.

I did two things in this example:

  1. Set drag handler for avoiding item was moving when I resize it.
  2. Add onResize callback: update item size when Resizable wrapper has changed.
  3. Tell this muuri grid instance, we need to re-layout the item positions, you can check out those codes at line 21 and line 22.
grid?.refreshItems();
grid?.layout();

@romanstetsyk
Copy link
Contributor Author

It's a good starting point, thanks!

@romanstetsyk
Copy link
Contributor Author

I've got another issue. If you could check it out when you have time
The issue is that DraggableGrid is rerendered unnecessarily, firing its layout method. It causes flickering and losing scroll position. I guess that's a muuri issue, not the react wrapper. I've set up a minimal example on Codesandbox. Would you happen to have any ideas on how to resolve it?

Screen.Recording.2023-06-10.at.2.36.47.AM.mov

@vingeraycn
Copy link
Owner

As I can see, the DraggableGrid has re-rendered because of the state called items has changed, the children prop of DraggableGrid has changed, it would be re-render. I think this is react way

However I add memo for grid, it seems like solve this issue. And there is an example forked from your codesandbox above, check it out here

Hope that helps you

@romanstetsyk
Copy link
Contributor Author

useMemo is key to preventing rerendering! I had to disable react-hooks/exhaustive-deps ESLint rule to make the grid not dependent on the state, then update the state and use muuri's native methods like move(), add(), layout(), etc. to manipulate grid items (which I was trying to avoid, tbh).
Here is an updated codesandbox

@vingeraycn
Copy link
Owner

Let's talk back at this issue. I have an idea, how about that we can store this scroll position before update items, and restore its scroll position after items have updated.

@romanstetsyk
Copy link
Contributor Author

@vingeraycn I think I've found a simple solution. I added this CSS snippet:

.ruuri-draggable-grid {
  height: calc(1px / 0);
}

It sets the height to 'infinity'. You can set a large value in pixels instead, like 10000px, but the scroll position will be lost again with a large number of grid items.
Would you like me to make a PR? I tested it in Chrome, Safari, and Firefox, and it works fine.

@romanstetsyk
Copy link
Contributor Author

I've got an issue when removing items from the grid and would greatly appreciate it if you could check it out.
Basically, if the event handler that removes the item is inside the item itself, it won't work. Here is the codesandbox. Clicking x in the grid (left column) won't remove items for some reason.

@vingeraycn
Copy link
Owner

vingeraycn commented Jun 15, 2023

@vingeraycn I think I've found a simple solution. I added this CSS snippet:

.ruuri-draggable-grid {
  height: calc(1px / 0);
}

It sets the height to 'infinity'. You can set a large value in pixels instead, like 10000px, but the scroll position will be lost again with a large number of grid items. Would you like me to make a PR? I tested it in Chrome, Safari, and Firefox, and it works fine.

No thanks, I think this behavior is not suitable for ruuri as a library, the library shouldn't do any hacks as little as possible for code robustness.

however, it's very appreciate for your tests. :)

@vingeraycn
Copy link
Owner

vingeraycn commented Jun 15, 2023

I've got an issue when removing items from the grid and would greatly appreciate it if you could check it out. Basically, if the event handler that removes the item is inside the item itself, it won't work. Here is the codesandbox. Clicking x in the grid (left column) won't remove items for some reason.

I reproduced this problem, which looks a little strange, like the problem of draggable-items cache control inside muuri.

Possible related issues:

@vingeraycn
Copy link
Owner

vingeraycn commented Jun 15, 2023

I've got an issue when removing items from the grid and would greatly appreciate it if you could check it out. Basically, if the event handler that removes the item is inside the item itself, it won't work. Here is the codesandbox. Clicking x in the grid (left column) won't remove items for some reason.

I found some issues, please wait for my news, I am trying to fix sync problem between data model and views.

Update: I am work on this pr #12

@vingeraycn
Copy link
Owner

I've got an issue when removing items from the grid and would greatly appreciate it if you could check it out. Basically, if the event handler that removes the item is inside the item itself, it won't work. Here is the codesandbox. Clicking x in the grid (left column) won't remove items for some reason.

I found some issues, please wait for my news, I am trying to fix sync problem between data model and views.

Update: I am work on this pr #12

Hi @romanstetsyk , could you try the new version v2.0.0-0 of ruuri? I wanna to know this version whether solve this problem(#11 (comment)) or not.

Tips: the new version has some break change, learn more at here

Thanks a lot.

@romanstetsyk
Copy link
Contributor Author

@vingeraycn, thanks for the update!
Do you have any idea why the grid here is not showing up? It uses version 2. I must have missed something, but I can't identify the issue

@vingeraycn
Copy link
Owner

vingeraycn commented Jun 19, 2023

@vingeraycn, thanks for the update! Do you have any idea why the grid here is not showing up? It uses version 2. I must have missed something, but I can't identify the issue

I got it! I missed to emphasize the uni key field in data property have to be string type in README.md and CHANGELOG.md. the example above used id(number type) as uni key.

BTW I organized code, and released v2.0.0-1, you can try this version if you have interest, thanks for your helps.

@romanstetsyk
Copy link
Contributor Author

@vingeraycn I've set up a new codesandbox to test v2.0.0-1. Unfortunately, it doesn't solve the problem of removing grid items entirely. There are still some sync issues. I also noticed a weird behavior with the container height that you can see on the recording below

Screen.Recording.2023-06-20.at.1.09.30.AM.mov

@vingeraycn
Copy link
Owner

@vingeraycn I've set up a new codesandbox to test v2.0.0-1. Unfortunately, it doesn't solve the problem of removing grid items entirely. There are still some sync issues. I also noticed a weird behavior with the container height that you can see on the recording below

Screen.Recording.2023-06-20.at.1.09.30.AM.mov

I try to fix those issues, and I have tested your examples above. Now v2.0.0-2 is available.

@romanstetsyk
Copy link
Contributor Author

I appreciate your fast reply! I've updated ruuri to v2.0.0-2 in the previous example, but the issues seem to remain

@vingeraycn
Copy link
Owner

vingeraycn commented Jun 21, 2023

It looks normal about model-view sync issue.
https://github.com/vingeraycn/ruuri/assets/19839331/38bd9f6d-5388-478e-bf57-735afbbea7c4

There are some infos about issue:

it doesn't solve the problem of removing grid items entirely.

I tried to remove DOM element when data was changed by using grid.remove API from muuri, but it accidentally throws an error which execute removeChild failed, can't read property of null from muuri inside.

And I followed error stack, found the error came from code itemElement.parentNode.removeChild..., the itemElement.parentNode is null, I feel confuse, I need to dig into muuri why I use this option removeOptions.removeElements = true will throw that error accidentally if I have time.

Back to now, the removeElements option of grid.remove API from muuri is default to false, and ruuri is based on this default action now.

Thanks! @romanstetsyk

@romanstetsyk
Copy link
Contributor Author

Actually, removing items work fine with v2.0.0-2! I've just checked the codesandbox I had created previously. It might have been some cache issues when I updated to 2.0.0-2, but it still used the old version.

@vingeraycn
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants