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

feat: add getContainerSize #621

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add getContainerSize #621

wants to merge 1 commit into from

Conversation

livwvil
Copy link

@livwvil livwvil commented Jun 26, 2023

Hi!
I am building a docking manager on top of this library and have run into a problem. I need to know the container size when Allotment onChange invoked.
Actually I need the width, height x and y of each panel inside the Allotment, but this is enough for me to find all the parameters myself.
If you want, I can add an advanced story next to VS Code with a docking manager.

@netlify
Copy link

netlify bot commented Jun 26, 2023

Deploy Preview for allotment-website ready!

Name Link
🔨 Latest commit 1d6cb88
🔍 Latest deploy log https://app.netlify.com/sites/allotment-website/deploys/649a9d0371404900083a904e
😎 Deploy Preview https://deploy-preview-621--allotment-website.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.

@netlify
Copy link

netlify bot commented Jun 26, 2023

Deploy Preview for allotment-storybook failed.

Name Link
🔨 Latest commit 1d6cb88
🔍 Latest deploy log https://app.netlify.com/sites/allotment-storybook/deploys/649a9d03d9954f0009d0dd87

@livwvil
Copy link
Author

livwvil commented Jun 27, 2023

I reverted the fix deps commit.

@johnwalley
Copy link
Owner

Hi @livwvil. Thanks for taking the time to describe your use-case and put together a PR.

I wonder if we can achieve this without any changes to the library? If we were to attach refs to any Allotment and Allotment.Pane components we could then determine the dimensions ourselves. I'll see if I can put together an example.

Note that this would rely on #582 being fixed first.

@johnwalley johnwalley added the question Further information is requested label Jun 27, 2023
@livwvil
Copy link
Author

livwvil commented Jun 27, 2023

The main problem is performance. I can get the size of the container by wrapping Allotment with another div and calling getBoundingClientRect on it. But it is slow operation, besides, the dimensions of the container are already known inside Allotment, there is no need to do double work.
Yes, I saw an issue with the Allotment.Pane ref. Seems like the root of it in cloning ReactChildren.
When I analyzed the code, I also noticed that PaneView.element is unnecessary (dead code). Maybe I am wrong.

@livwvil
Copy link
Author

livwvil commented Jun 27, 2023

Take a look at the DockingManager story in docking-manager-story branch in my repo for more details about the problem.
Anticipating the question why, I will answer. I need absolute panes because the structure of the tree can change, but panes stay the same, I have to prevent remount in the DOM and preserve pane's component state and scroll position out of the box (without any portal tricks).
My previous attempt to preserve state is not perfect. Unfortunately, I'm losing the scroll position. Having peeped the solution in other libraries, I realized that I needed absolutely positioned panels.

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

Successfully merging this pull request may close these issues.

2 participants