Ensure reflow reflows non-visible widgets, fix scroll_to_center issue #2684
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
darrenburns
changed the title
|
Is that all it was?! I thought it would be a much more complex issue than that. |
|
@willmcgugan Yup, seems like it 😄 |
darrenburns
commented
May 29, 2023
| yield Label(("SPAM\n" * 49)[:-1]) | ||
| yield Label(("SPAM\n" * 51)[:-1]) | ||
| yield Label(("SPAM\n" * 59)[:-1]) | ||
| yield Label(("SPAM\n" * 53)[:-1]) |
There was a problem hiding this comment.
Simplified this a little because I was struggling to understand it vs what was on screen. It still has multiple levels of nested scrolling containers, both vertical and horizontal.
willmcgugan
approved these changes
May 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2268
Closes #2254
When
reflowwas called, we were only reflowing visible widgets. This was preventing widgets that were not visible from being added intofull_map.reflow_visiblewas explicitly passingvisible_only=Truetoarrange_root, but that was actually already the default value.reflowwas not passingvisible_onlyat all, so it was defaulting toTrue.This broke
scroll_to_center, because widgets that were scrolled out of view were not present in the compositors full map.With this change, the
_full_mapis now fully populated byreflow, andscroll_to_centerworks as expected.I've removed the
xfailthescroll_to_centersnapshot test and it seems to pass consistently now.Please review the following checklist.