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

.isCollapsed() seems very unreliable? #325

Closed
oskarengstrom opened this issue Mar 23, 2024 · 7 comments
Closed

.isCollapsed() seems very unreliable? #325

oskarengstrom opened this issue Mar 23, 2024 · 7 comments

Comments

@oskarengstrom
Copy link

oskarengstrom commented Mar 23, 2024

I'm calculating the collapsedSize of a panel to enable px-based values, and running into some issues related to that. It seems, when using this hook to supply a value to collapsedSize .isCollapsed() is always returning false. I can run .collapse() successfully on a ref of the panel, but then .isCollapsed() returns false.

First my guess was that the number had too many decimals (there seems to be an issue with more than 10 decimals), but toFixed() is not solving my OG issue.

So then I was guessing - if the fact that my hook returns one number on first render, and then another on second, could be the issue? This seems more likely.

I will continue testing on my end, but would be nice to hear if anyone has any ideas on what might be wrong?

(v2.0.13)

@bvaughn
Copy link
Owner

bvaughn commented Mar 23, 2024

Please share a repro. It's really hard to help trouble shoot issues where there's no shared code.

I suggest you also check out #265 and #264 since they seem to be related. Generally speaking, I have decided not to support pixel based constraints, so what you're doing might just be out of scope for this library, but I can't tell without seeing a repro.

@oskarengstrom
Copy link
Author

Here's a repro: https://codesandbox.io/p/sandbox/react-resizable-panels-forked-pydfkc

As you can see the left panel (using the minSize-hook to generate the value for collapsedZise) isn't expanding, but the right (which uses an integer value) can indeed expand.

@bvaughn
Copy link
Owner

bvaughn commented Mar 23, 2024

I recorded a Replay of the repro you provided, and I can see where the isCollapsed method is returning something other than what you're expecting. This is happening because the panel's actual size (3.8764385221) is not the same as its collapsed size (3.8764385221078133). (So it's basically a precision issue.)

I try to account for potential precision issues within the library's own methods, but this case kind of falls between the cracks a bit, because useMinSizePx is passing a more precise value than the library expects. (Things like this are one of the reasons I decided not to support pixel based constraints.)

I guess I could account for this by using a fuzzy comparison in the is-collapsed/is-expanded helper methods.

@bvaughn
Copy link
Owner

bvaughn commented Mar 23, 2024

2b0ff1a should guard against high precision values from code like the example you shared. It has been released in version 2.0.14.


❤️ → ☕ givebrian.coffee

@bvaughn bvaughn closed this as completed Mar 23, 2024
@oskarengstrom
Copy link
Author

Thanks a lot @bvaughn! 🙏

I did notice that the props onCollapse and onExpand stopped working now 🤷‍♂️
Updated the repro:
https://codesandbox.io/p/sandbox/react-resizable-panels-collapsedsize-bug-pydfkc?file=%2Fsrc%2FApp.js%3A42%2C37

@bvaughn
Copy link
Owner

bvaughn commented Mar 24, 2024

The number of edge cases with high precision values are part of the reason I chose not to support pixel based constraints.

2.0.15 should fix the callback issue. If you uncover anything else though, you'll need to submit a PR. I don't think I have the mental energy to try to support this use case 😅 Thanks for understanding.


❤️ → ☕ givebrian.coffee

@bvaughn bvaughn reopened this Mar 24, 2024
@bvaughn bvaughn closed this as completed Mar 27, 2024
@bvaughn
Copy link
Owner

bvaughn commented Mar 27, 2024

Think I accidentally re-opened this one so I'm going to close it now. We can talk more on a new issue if anything else crops up.

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

No branches or pull requests

2 participants