Skip to content

Fix traversal order issues caused by intersecting or overlapping child rects - #21235

Merged
jonahwilliams merged 4 commits into
flutter:masterfrom
jonahwilliams:fix_easy_bug
Aug 31, 2018
Merged

jonahwilliams merged 4 commits into
flutter:masterfrom
jonahwilliams:fix_easy_bug

Conversation

@jonahwilliams

@jonahwilliams jonahwilliams commented Aug 30, 2018 •

Copy link
Copy Markdown
Contributor

Fixes #17375

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Even in your moment of triumph you'll have to add a test 😃

List<SemanticsNode> _childrenInDefaultOrder(List<SemanticsNode> children, TextDirection textDirection) {
final List<_BoxEdge> edges = <_BoxEdge>[];
// Using a small offset value to shrink child rects removes overlapping cases.
const Offset smallOffset = Offset(0.1, 0.1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also be done in sortedWithinVerticalGroup? However, rather than duplicating the logic in multiple places, can we Rect.deflate by a delta and use the same rectangles in all places?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added for the sortedWithinVerticalGroup. Since we get the rects as is from the semantics nodes I don't think we can/should mutate them there.

@jonahwilliams

Copy link
Copy Markdown
Contributor Author

LGTM for real?

@yjbanov

yjbanov commented Aug 31, 2018

Copy link
Copy Markdown
Contributor

lgtm

@jonahwilliams
jonahwilliams merged commit f2442b8 into flutter:master Aug 31, 2018
@jonahwilliams
jonahwilliams deleted the fix_easy_bug branch August 31, 2018 02:55
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected traversal order on home screen of new gallery

4 participants