-
Notifications
You must be signed in to change notification settings - Fork 328
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
Min and Max column width not behaving as expected #1622
Comments
This is not necessarily a bug, or a feature request - it's more tracking for research into what's going on / how to improve the table column experience or even how to just make docs about table better |
Hi @mtanco! can I please work on this issue? |
Hi @mtanco ! I'm currently a student and new to open source. I would like to take up this issue with my partner @magandren ! |
Go for it @sabrinaxiao! |
Thank you for this opportunity @mturoci! We have a quick question in regards to completing this task. From our understanding, it seems that we are given 4 days in order to complete the task before it is given up to others that are interested in working on it. We both are in a class where our last assignment is making a contribution to an open source project. For our assignment, we will be writing up a report in regards to the project and task we selected along with our planning process by the end of this week. We also are asked to write a second report at the end of this month on our findings and if we were able to complete our task using software engineering concepts we learned in class. As students who may not have enough time to make a meaningful contribution within a couple of days, we were wondering if we can have more time to work on this task. Thank you so much for your time and flexibility! |
This was just for the Hacktoberfest that is already passed. No more time limits from now on so feel free to take as much time as necessary :) |
@mturoci Hello! We have some initial clarifying questions in regards to our expected behavior to ensure we going in the right direction!! After running the code, we get the following tables:
Once again, thank you so much for your help! |
You can propose what you think would be ideal. IMO if
I don't think it's possible to compute the "ideal" width, e.g. the width of the longest string in a column because the table can be paginated, have wrapping set etc. + it would be very slow.
Again, proposals are welcome. IMO min/max should be primarily used for column resizing via mouse and:
|
@mturoci We have adjusted our code locally such that min_width = 0 would appear the same as a table without a min_width specified. Based on our findings, our default value for min_width is 150px and our default value for max_width is undefined. We noticed that the current behavior of the code matches these two cases you specified: If min is set and it's > default, then use min Just to clarify, is the expected behavior of: “if min < default < max, use default.” result in both min_width and max_width to be equal to 150px? Also regarding testing our code changes, where would you like to include the test cases to ensure our results are correct? Is there a format or a resource to follow as to how these test cases should look like? |
Not sure what you mean exactly. It means that if min=100, default=150 and max=200, the column should be 150px initially and allow resizing up to 100/200 (min/max).
Plain PR screenshots with minimal python example code should be a good start. Ideally, you might want to include the tests within our table unit tests ( |
I see our confusion my apologies! We primarily looked into table.tsx and found how to manipulate the values of min_width and max_width. We currently are having trouble understanding how the widths of the table columns are set. Do you have any suggestions/guidance on where to look? Thank you so much again. |
When approaching a problem in general, it's good to start by looking at how current implementation works, understand it and then do the necessary changes. You can trace the min/max props and see where and how they are used, then think of how to alter the behavior according to your desires. Wave uses Fluent component library under the hood, so you might look there as well. |
@mturoci Thank you so much for this advice! We looked into the Fluent component library and found that the behavior of why “min_width = 0 is bigger than expected” (it’s being set to 100px by default) lies in the code logic of FluentUI here . In our findings, we looked into the behaviors of min_width and max_width. We found that if min_width = 0 or undefined and max_width was set, according to the logic of FluentUI, the min_width will be set to the max_width. This explains the unexpected behavior in which “min_width seems to be ignored if max_width is set”. In addition to the behaviors we were assigned to investigate, we also found an additional unexpected behavior. In FluentUI, if the max_width is set, the variable is not utilized at all. As a result, despite the max_width being set, the user is still able to stretch the table column width indefinitely with no restriction. As an example, in one of the unit tests in FluentUI, you can see here that the max_width = 400px however this unit test tries to set the max_width to 500px. Potentially in the future, this feature will be fully implemented in FluentUI. As most of these unexpected behaviors are due to FluentUI, as a temporary solution for the “min_width = 0 is bigger than expected” behavior, we decided to change min_width such that if the user would like to set min_width = 0px, it will be displayed as a non zero value (0.001px) which matches our expected behavior. Making this quick fix also resolved the unexpected behavior of “Tables are identical when using max_width regardless of if min_width is set or not, in that min_width seems to be ignored if max_width is set”. The min_width is no longer ignored with this fix. We have submitted a pull request of our changes to reflect this min_width change. This concludes our investigation! Any feedback is greatly appreciated!!! |
Wave SDK Version, OS
h2o_wave==0.22.0
Actual behavior
The goal is to programmatically make columns widths automatically the right size to fit in a table without scrolling. Upon trying to do this we found some interesting and maybe unexpected behaviors:
min_width==0
is bigger than expectedmax_width
regardless of ifmin_width
is set or not, in thatmin_width
seems to be ignored ifmax_width
is setThe text was updated successfully, but these errors were encountered: