-
-
Notifications
You must be signed in to change notification settings - Fork 149
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
Support pixel-based panel constraints #176
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
7b5f478
to
23d63d5
Compare
Just wanted to mention that it seems like using min/max widths on the Granted, I am using the shadcn implementation here: https://ui.shadcn.com/docs/components/resizable |
But if you set |
Can you say more about this? I'm not sure what you mean by "resizing in the background", do you mean programmatically? |
For example, first apply the following css or style to a panel:
Then use the resize handle to expand the width of the panel. Once the panel width reaches 500px, the element will (correctly) not change in size. However, as you drag past 500px, or if you continued to attempt to drag the resize handle to increase the size, the actual width of the panel (within the library) is > 500px. You could check this by logging from the In summary, if you drag to 650px, the visual element correctly stops expanding at 500px. However, when you drag to decrease the size, you will have a 150px dragging delay before the element starts reducing in width. @bvaughn This one hurdle could be resolved by having the ability to programatically interrupt resizing a panel. This way, for example we could have something like:
I don't know if this is simple or complex, as this would depend on the implementation within the library. Even with the library currently as is, I could envisage this being solvable - by calling the imperative |
Bravo on illustrating the issue. For folks in the future: |
@zac18992 I've intentionally decided not to support pixel constraints, as mentioned in the README:
I haven't thought through the implications of a prop like |
You're right to call out the downside of the approach discussed above though! 👍🏼 |
@piszczu4 @lacymorrow This issue is very important to my use case. I believe I have found my own solution to prevent this, based on the above, but I will try to finalise it before sharing an example, to check if there are any other issues. @bvaughn I do believe that However, I'm not saying I expect you to implement this. Of course you need to limit the scope so you can be productive. Thanks for replying! |
Thanks for understanding, @zac18992 🙇🏼
Happy to link to your solution in the FAQ as well! |
While I understand the desire to keep things simple and easy to work on, removing support for pixels is major pain point for a SAAS project my team is working on. We've resorted to downgrading to v0.0.63 so we can continue using pixel and percentage values. However, we've recently seen some runtime errors ( Thankfully, I happened to look through this issue when I did and found @zac18992's solution. I'm hopeful that your solution will work for our use case and maybe even address the runtime errors as well. I'll give it a shot and report back. |
@mrmattrc I'm sorry to hear that your project was negatively impacted by this decision. This is a side project for me though, done pro bono, and I have to keep scope manageable if I want to continue to support the project at all. If this is a commercial use case, we could always chat about the possibility of me doing some for-hire work. |
My use case is always to use responsive layouts (with percentages) but requires min and max values in PX for panels. Here is an example of how I got this to work with the library as it currently is. This rectifies the issue and prevents panels from extending past their designated max/min span, which is set using an inline style. It may have some bugs or performance considerations. I haven't explored using the library with purely PX values, but I think the above covers the majority, if not all of those use cases. React Context could be used for passing the internal values for the min/max mechanism, if preferred. Like I said, this should probably be in the library and would probably be a lot easier to implement, and more performant, if done so. But sounds like @bvaughn needs convincing and then someone needs to create a PR :) Hope this helps some people! |
Code examples are good, if they've been vetted. I'm willing to link to them in the FAQs if they'll help others (again, only if they've been tested and seem to work well). But I want to set expectations here regarding feature support for this. I believe that supporting pixel based constraints in this library would be a lot of work (both to implement initially and to maintain over time) and I am not willing to do it pro bono. If someone wants to talk about some sort of for-hire arrangement then I'm willing to consider it but I'm not really interested in anything short of that. I hope you'll all understand that my free time is limited, and it's really important to me to have a good balance between sharing libraries like this and not burning myself out trying to add features for other people (particularly in support of for-profit use cases). |
I haven't looked at all the edge cases. But from first glance, I do believe the support could be simpler than it seems. Especially as it can already be fudged externally using the existing version. It could maybe be added with a prop allowing for control over whether a resize can happen or not. Or probably using a few other approaches as well. Anyway, I'm currently happy with what I have. If I want to add it to the library, maybe for performance reasons, then I will just fork it and share, as it sounds like this is not something that will be considered. Thanks for all your great work on this library. I really appreciate it! |
Having tried to add support already, I think you're under estimating the effort for this. You don't just need to make per-panel decisions at the time of a resize / mouse event, you also need to listen for size changes and re-evaluate panel constraints for the entire group (which can get really complicated). That being said, fork and share sounds like the way to go. |
I can totally see that! Like I said, I haven't looked at all the edge cases, and am not pushing any agenda as I believe I have it working as I desire already :). In my case, I believe the panel constraints are handled by the existing percentage based logic. |
@bvaughn, re-reading my comment, I feel like I was more harsh than I intended. It's great having a library to handle this functionality, and I really appreciate all your work! We are a small and early-stage start-up though, so we're pretty restricted as far as sponsoring or for-hire work. It's not perfect, but it looks like the solution suggested here should work pretty well for us! We are running into this minor issue:
I agree with @zac18992 that adding something like a |
Hey @mrmattrc @lacymorrow @piszczu4 check the code sample in my above comment. It could be of use :) |
@mrmattrc No worries! 😄 I appreciate your comment though! I'll let the Report back with the approach that's being discussed above. If it works out well for you all then I can add it to the FAQs. |
In case anyone is looking for this here's how I managed to do it w/ shadcn. Some glitches still may happen though and it most likely doesn't handle every edge case. |
The README says that panel sizes cannot be specified in pixels and indeed units="pixels" does not work. So the fact that this PR was merged, and these docs exist is confusing |
@DawidWraga This PR and the CI build for this branch are 9 months old. Pixel-based constraints was something I tried and abandoned when it became clear that there were too many edge cases for me to support them correctly. (This is a side project for me. No one is paying me to build those features.) If you want to use an older release that had pixel based constraints, you can. |
Relates to issues #46, #47, #51, #78, #114, #128, #141
This PR adds a new prop (
units
) toPanelGroup
. This prop defaults to "percentage" but can be set to "pixels" for static, pixel based layout constraints.This can be used to add enable pixel-based min/max and default size values, e.g.:
Imperative API methods are also able to work with either pixels or percentages now. They default to whatever units the group has been configured to use, but can be overridden with an additional, optional parameter, e.g.
See the docs for more: .../examples/pixel-based-layouts