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

Removed wrong saving of layout #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

filippo-biondi
Copy link

Removed a saving of layout in responsiveGridLayout(). The saving was happening before findOrGenerateResponsiveLayout and this caused the originalLayout to be saved as layout for the current breakpoint. This caused some plots to overlap if the number of column in the originalLayout was bigger than the number of columns allowed by the current breakpoint. Furthermore the layout is correctly saved after findOrGenerateResponsiveLayout so I think the efficency is unaltered

Copy link

netlify bot commented Nov 9, 2023

Deploy Preview for grid-layout-plus ready!

Name Link
🔨 Latest commit 4991ac7
🔍 Latest deploy log https://app.netlify.com/sites/grid-layout-plus/deploys/654d25be8a415e000808e687
😎 Deploy Preview https://deploy-preview-12--grid-layout-plus.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nastita
Copy link

nastita commented Aug 15, 2024

@filippo-biondi does this fix the overlaps when using responsive?

At least in the deployed netlify version it doesn't seem so.

SCR-20240815-jnpf

@filippo-biondi
Copy link
Author

Yes you're correct. Probably my fix solved only the specific issue I was having on my project. I think the root cause of the overlapping is the implementation of findOrGenerateResponsiveLayout.

@nastita
Copy link

nastita commented Sep 6, 2024

@filippo-biondi I bypassed this by making my own items positioning logic based on columns size, but Ideally I think it should be fixed in the library.
I will check the implementation of that function and debug it if I find some time.
I don't know if @qmhc is still maintaining the library though.

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

Successfully merging this pull request may close these issues.

2 participants