-
Notifications
You must be signed in to change notification settings - Fork 96
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
FIX Add one to current colspan in fixColumns instead of resetting to 2 #1869
FIX Add one to current colspan in fixColumns instead of resetting to 2 #1869
Conversation
ee22a35
to
ae3f7db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Couple of small changes
ae3f7db
to
8d14019
Compare
8d14019
to
3d6b2d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#1869 (comment) hasn't been responded to. We're assuming if the colspan wasn't defined, it is already 1 and should become 2. Does that always hold true? Can we do something like count the number of columns in the table or something?
The calling function adds one column to the table, so a correctly set up table will always have rows with the number of columns minus that new column. My change changes a hardcoded 2 that disregards the existing colspan to something that takes it into account. Counting the number of columns isn't needed, the entire purpose of this function is to add one column. If this leads to a wrong number of columns, there's a near certainty that said table already has a wrong number of columns in the current release. As far as I can tell, the only native integration that's affected by this is the 'No items found' row. In my eyes this is legacy code that should be removed altogether, but I'm not entirely sure if that comes with complications or not. |
3d6b2d9
to
c297bef
Compare
c297bef
to
807026d
Compare
In the previous situation, the 'No items found' row would be hardcapped at colspan=2. This change ensures that that row always spans all columns in the table
807026d
to
7ac6a72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks! Just letting CI run and I'll merge if it's green.
Description
In the old situation, the final column in each row would be hardcoded to colspan="2" if the function 'fixColumns' was called. This is required because this function adds an additional column to the table that needs to be filled in. This change removes the hardcoded number and instead adds one to the current colspan of the last cell. This fixes the bug that the 'No items found' row would be capped at two columns even if the table has more.
Manual testing steps
Issues
Pull request checklist