fyne icon indicating copy to clipboard operation
fyne copied to clipboard

fix: top Table items were invisible after window resize

Open cmitsakis opened this issue 2 years ago • 4 comments

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.

cmitsakis avatar Jun 03 '22 21:06 cmitsakis

Coverage Status

Coverage increased (+0.008%) to 61.367% when pulling 858aaeb0f615ec23cc42c21f2433981a08b73405 on cmitsakis:fix/3034 into f01b3d496f6d910dc94b8d1548877d9fabc1bd09 on fyne-io:develop.

coveralls avatar Jun 03 '22 21:06 coveralls

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.

cmitsakis avatar Jun 08 '22 19:06 cmitsakis

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.

cmitsakis avatar Jun 11 '22 09:06 cmitsakis

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.

Jacalz avatar Jun 16 '22 17:06 Jacalz

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.

andydotxyz avatar Apr 12 '23 13:04 andydotxyz

This fix was covered in a separate PR at some point, closing this change.

andydotxyz avatar May 15 '23 13:05 andydotxyz