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

Add row resizing for tables #50

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

Conversation

ColRealPro
Copy link

@ColRealPro ColRealPro commented May 10, 2024

Fixes rows not resizing when at least one column has an element bigger than others inside the same row.

image

@ColRealPro ColRealPro changed the title Add demo for resizing rows in demo window Add row resizing for tables May 10, 2024
@ColRealPro
Copy link
Author

Just noticed an oversight in where if the elements inside are updated, or the window size is updated, it won't resize. I'll try to fix this tomorrow likely

@SirMallard
Copy link
Collaborator

Hi, thanks for this. I'm having a look at the moment

From what I've read, and can try to understand, the first code finds all cells on the same row by using the LayoutOrder property. Is there a reason the update code is run once for every column, rather than at the end once all the columns are ready? There seems to be a lot of looping to determine the rows, which includes a lot of nested for loops and I'm sure these could be simplifed down, especially since they rely on :GetChildren() and sorting through tables.

Then, every frame, it loops through all the cells and updates the rest of the cells if the height is exceeded. As a far as I can see with this code, you loop through all the cells and update every other cell (for loop in a for loop), rather than checking the maximum height first and then updating them all at once? Is there a reason for this?

Also, we are going to do a large table redesign for the next update, since they are fairly limited as you have found and there are many edge cases. From my personal opinion of the best way to do this, I would suggest we change it to be row-based, which automatically fixes these issues, because we can then have greater control over the width of the columns. That's not to say that this isn't a helpful fix, but that it might become obsolete with a total table redesign.

Would you be able to explain through the code, just so I have a better understanding of your solution? Many thanks.

@ColRealPro
Copy link
Author

Hey Mallard, I have yet to actually try to improve on a lot of this yet, so thanks for pointing out a lot of these issues with the nested for loops, I never had a reason for a lot of them other than I didn't know the best way to approach it at the time.

As for the table redesign that you mentioned, I would agree that a row-based system would be much better in preventing this, especially since from what I've seen, the width of columns is static and doesn't autoscale.

I will try to improve on a lot of this when I get home later today and hopefully make the code a lot more clean and easy to understand. I'll explain my solution for it after the improvements as well.

@ColRealPro
Copy link
Author

Ready for review! I've cleaned up the UpdateCellSizes function that is run every frame and moved it out of the for loop where the columns are made.

On top of that, I've added some optimizations so the cells are only updated when they need to be, which happens when either a child is added to the table, or the window is resized. I'm not sure if my method for checking if the window has resized is the best approach though as it feels sort of hacky, however I couldn't get the onChange() function to work properly, would love to see if there's an easier method though.

How it works now is as follows:

  • First it will store all cells in every column into a table along with their LayoutOrder, and reset all the cells sizes during this loop so they can be resized down if needed, also during this loop it will find whichever column has the most rows inside it, which is used later on in the process
  • Second it will then sort through this table by their layout orders from least to greatest
  • Thirdly it will go through the cells of the column with the most rows, and go through other columns, and grab the cell on the same row, and then compare the two and store whichever is biggest, and at the end set all of them in that row to whichever is biggest

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