VirtualizedFX icon indicating copy to clipboard operation
VirtualizedFX copied to clipboard

Issues on cell size changes

Open Alexander-Ploskin opened this issue 2 years ago • 2 comments

Changes:

  1. Fix exception, when after cell size changed, columns range shifts (for example, [2, 3] -> [3, 4]). On that line cells collection can't contain rIndex, because cells, which corresponds to any columns from the columns range was removed earlier.

  2. setPosition(0, 0) causes a lot of problems to me (redundant frame updates). I didn't find any purpose for this behavior. I removed this line and tested in my project, and it looks fine. Could you please describe why it actually needed?

Alexander-Ploskin avatar Mar 28 '23 22:03 Alexander-Ploskin

  1. Yeah that must have been a type, thanks for the fix
  2. I'm reluctant on changing that code fragment. If I wrote that, there must have been a reason. I don't actually remember why exactly, but what that code does is reposition the flow at [0, 0] if the cells' size changes and the layout was updated In other words, it should be a 'better safe than sorry' solution to avoid unwanted states for edge cases situations. In fact, the same happens for VirtualTable and their paginated versions

The thing is that you are also trying to implement a rather special case. A VirtualFlow must have fixed cell sizes at all times. When you are zooming, you are practically "cheating" on this rule. And because of this rule, my way of seeing cell sizes changing is: if size has changed, then it's not safe to keep the flow at the same position it was before because anything can happen.

  • Maybe the position is still valid, but will display other element
  • Maybe the position is invalid, could be greater than the current estimated size Taking into account these cases makes the code and maintainability much harder

palexdev avatar Apr 08 '23 15:04 palexdev

What we could try to do is to add a flag to avoid that code fragment from triggering. In other words, a way to disable that system is special cases like yours, or even simpler, just override the method since you can do it

On the other hand... It would be nice to not reset the position, but at least ensure that it is valid before doing anything. In pseudocode, something like this:

		if (getWidth() != 0.0 && getHeight() != 0.0) {
			if (!manager.init()) {
				requestViewportLayout();
			} else {
				// Check Positions
                               // Ensure both vPos and hPos are within the new estimated sizes
                              // This can be done with clamping
                             // After clamping set the positions
			}
		}

Mind testing this, maybe send me a "code preview"?

palexdev avatar Apr 08 '23 15:04 palexdev