-
Notifications
You must be signed in to change notification settings - Fork 28
SHIELD-13816 - fix defects of resizing grid #6502
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
components/table/table-wrapper.js
Outdated
| for (let i = 0; i < candidateRowHeadCells.length; i++) { | ||
| candidateRowHeadCells[i].style.minWidth = ''; | ||
| candidateRowBody.cells[i].style.minWidth = ''; | ||
| } |
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.
Best to merge this into the for loop just below; otherwise we're looping through the head cells twice.
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.
Notably, the for loop below already fetches the i'th head cell and the i'th body cell, so you should be able to move this logic below those two fetches.
|
Thus far, only the resizing defect has been fixed, the flickering defect still happens. |
|
Resetting the minimum width when we do the sizing calculation seems fine (and safe enough) to me. I'm not sure what kind of cost we may incur in doing so, but it might be unavoidable. |
|
The current code in this PR fixes the main problems that were happening: the flickering when the width of the grid was close to that of the page (caused by a render loop), the misalignment of columns when resizing, and the mis-sizing of columns when resizing. There is a small defect left, which is in the comment/video below. |
Updating vdiff goldens for PR 6502
|
Testing: After the change, both of the above defects no longer happen. The width resizes automatically, and the flickering does not happen. When testing, I found another defect of the scroll not being able to go all the way to the right, so I changed the code and retested, and now all of those defects are fixed. These changes did result in 2 new small defects (see above comment) which Elaine is aware of. |
…UI/core into nplett/core/fix/SHIELD-13816
Updating vdiff goldens for PR 6502
Updating vdiff goldens for PR 6502
Are these defects considered acceptable or are you fixing them in other PRs? |
|
There is a larger defect of these changes not allowing the grid to be scrolled to the right in some instances. These fixes are on hold for now, and will be revisited in the future. |

Fixing a defect for SHIELD-13816