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

Mark pod correctly for view sequence #186

Closed
wants to merge 1 commit into from
Closed

Conversation

IsseW
Copy link

@IsseW IsseW commented Mar 10, 2024

From what I can tell only the last pod in a view sequence was correctly marked when ChangeFlags::Paint was requested. This should mark the correct pod and not just the last one.

@Philipp-M
Copy link
Contributor

Have you noticed any issues?

The elements.mark() is necessary, otherwise no tree-structure updates go through.

This code should be run per view, and elements.mark takes the current last Pod to mark changes (which should already be pod in that code).
Everytime mutate (a few lines earlier) is run, a new pod (current of the view) is added to the elements.

@IsseW
Copy link
Author

IsseW commented Mar 10, 2024

Have you noticed any issues?

The issue I noticed was for rebuild, where the current last pod always seems to be the last pod. If it's mplemented like it is for LinearLayout. Of course I could've missed something though, still pretty new to learning about how xilem works.

This change fixes that for me at least. Where the specific problem I had was an image not changing if I only returned ChangeFlags::Paint. And found out through some debugging that the paint was being sent to the last element in the sequence.

@IsseW
Copy link
Author

IsseW commented Mar 10, 2024

The elements.mark() is necessary, otherwise no tree-structure updates go through.

I can see this now with what TreeStructureSplice is doing. Only considered what VecSplice did earlier... But TreeStructureSplice still calls VecSplice which will only mark the last element.

@Philipp-M Philipp-M mentioned this pull request Mar 10, 2024
@Philipp-M
Copy link
Contributor

Can you check if #187 fixes the issue?

@IsseW
Copy link
Author

IsseW commented Mar 10, 2024

Closing in favor of #187. And moving discussion there

@IsseW IsseW closed this Mar 10, 2024
@IsseW IsseW deleted the testing branch March 10, 2024 11:22
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