Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove manual recursion to child paint and accessibility #557

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

tomcur
Copy link
Member

@tomcur tomcur commented Aug 26, 2024

Recursing is done inside the paint and accessibility passes since ff7635e. I believe this is the correct continuation of #522, with the removal of these methods "left for later" as mentioned in #522 (comment).

One note is that Flex now debug-paints its baseline under its children, rather than over them.

Recursing is done inside the paint and accessibility passes since
ff7635e.
Comment on lines -19 to -21
.paint_fn(move |child, ctx, scene| {
child.paint(ctx, scene);
})
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this get at child paint (and maybe accessibility) in some other way?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so - all that this paint implementation did was paint the child, which is now implicit/always the behaviour.

I think that this is fine.

@DJMcNab DJMcNab requested a review from PoignardAzur August 26, 2024 15:44
Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

This looks as expected, thanks.

I've done some thinking on the modular widget thing, and I think this is the correct translation. Thanks

Comment on lines -19 to -21
.paint_fn(move |child, ctx, scene| {
child.paint(ctx, scene);
})
Copy link
Member

Choose a reason for hiding this comment

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

I don't think so - all that this paint implementation did was paint the child, which is now implicit/always the behaviour.

I think that this is fine.

@DJMcNab DJMcNab added this pull request to the merge queue Aug 28, 2024
Merged via the queue into linebender:main with commit 923c0fb Aug 28, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants