fyne
fyne copied to clipboard
fix: top Table items were invisible after window resize
Description:
Fixes #3034
This appears to be working.
Unfortunately I couldn't write tests for this one because I don't know how to check for visible elements, but I decided to send the PR now hoping it will be merged before the next release.
The issue was happening only when the size of the table increased as a result of the window size increasing. So in that case only I call ScrollToTop().
Losing scroll position doesn't seem to be a problem. The top row appears anyway, by calling ScrollToTop() I make it visible. But there might be cases where it causes a small inconvenience.
Checklist:
- [ ] Tests included.
- [x] Lint and formatter run with no errors.
- [x] Tests all pass.
Coverage increased (+0.008%) to 61.367% when pulling 858aaeb0f615ec23cc42c21f2433981a08b73405 on cmitsakis:fix/3034 into f01b3d496f6d910dc94b8d1548877d9fabc1bd09 on fyne-io:develop.
In Table it's not possible to access the cells because they are stored in TableCellsRenderer. That's why I couldn't create a test (although I created a test for my fix in List where I could access the items).
But I found a workaround that works. I store every label created by CreateCell() in a slice and in the end I check the text of those labels. It seems to work although I don't know if it's a good idea and I don't really understand it.
I improved the test. I found out how to calculate the exact size that triggers this bug. Although when I run the test with -race it doesn't work as expected. very strange.
Also when I added this line
log.Printf("c.size.Height (%v) > cSizeBeforeResize.Height (%v)\n", c.size.Height, cSizeBeforeResize.Height)
in Resize() before I call c.t.ScrollToTop()
, I noticed that before I increase the size of the window c.size.Height = 109 and after I increase it c.size.Height = 110
So I'm wondering what would happen if I fixed the size to be 110 instead of 109. Not sure where this value is calculated.
I will look into it again tomorrow to see if I can improve my fix.
Please don’t force-push your changes. It messes with the commit history and make everything harder to review. We’ll squash when merging if we think it’s needed.
I just found this PR which has unfortunately got a bit behind the current state of table. Are you able to pick it up, resolve the conflicts and continue to the complete solution @cmitsakis ? I'm sorry we have not been more helpful in getting it landed already.
This fix was covered in a separate PR at some point, closing this change.