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

Showing is too volatile to live in ViewEnvironment #844

Closed
rjrjr opened this issue Jul 27, 2022 · 0 comments · Fixed by #845
Closed

Showing is too volatile to live in ViewEnvironment #844

rjrjr opened this issue Jul 27, 2022 · 0 comments · Fixed by #845
Assignees
Labels
ui Related to UI integration
Milestone

Comments

@rjrjr
Copy link
Contributor

rjrjr commented Jul 27, 2022

If ViewEnvironment.equals() returns false on a call to WorkflowRendering(), unwanted recomposition happens. Showing is always updated to include the latest rendering, which is 99% likely to be unequal to its predecessor (new event handling lambdas). It's way too late to replace all the event handling lamdbas in the world with an event object based mechanism, so I think I need to find a new home for Showing. Back to a view tag?

https://workflow-community.slack.com/archives/C013JGW58N7/p1658941768749279?thread_ts=1658939305.914679&cid=C013JGW58N7

@rjrjr rjrjr added the ui Related to UI integration label Jul 27, 2022
@rjrjr rjrjr added this to the ui-1.0 milestone Jul 27, 2022
@rjrjr rjrjr self-assigned this Jul 28, 2022
rjrjr added a commit that referenced this issue Jul 28, 2022
Using the `ViewEnvironment` to hold the current `Screen` and `Overlay` was conceptually very clean, but was bad for performance in Compose, which would recompose too often when a new `ViewEnvironment` was not equal to the previous one -- all due to `Showing` being updated. We now store the canonical `Screen` rendering for each `View` in a tag, and the `Overlay` for each `Dialog` in a tag on its decorView.

Fixes #844
rjrjr added a commit that referenced this issue Jul 28, 2022
Using the `ViewEnvironment` to hold the current `Screen` and `Overlay` was conceptually very clean, but was bad for performance in Compose, which would recompose too often when a new `ViewEnvironment` was not equal to the previous one -- all due to `Showing` being updated. We now store the canonical `Screen` rendering for each `View` in a tag, and the `Overlay` for each `Dialog` in a tag on its decorView.

Fixes #844
rjrjr added a commit that referenced this issue Jul 28, 2022
Using the `ViewEnvironment` to hold the current `Screen` and `Overlay` was conceptually very clean, but was bad for performance in Compose, which would recompose too often when a new `ViewEnvironment` was not equal to the previous one -- all due to `Showing` being updated. We now store the canonical `Screen` rendering for each `View` in a tag, and the `Overlay` for each `Dialog` in a tag on its decorView.

Fixes #844
rjrjr added a commit that referenced this issue Jul 28, 2022
Using the `ViewEnvironment` to hold the current `Screen` and `Overlay` was conceptually very clean, but was bad for performance in Compose, which would recompose too often when a new `ViewEnvironment` was not equal to the previous one -- all due to `Showing` being updated. We now store the canonical `Screen` rendering for each `View` in a tag, and the `Overlay` for each `Dialog` in a tag on its decorView.

Fixes #844
rjrjr added a commit that referenced this issue Jul 28, 2022
Using the `ViewEnvironment` to hold the current `Screen` and `Overlay` was conceptually very clean, but was bad for performance in Compose, which would recompose too often when a new `ViewEnvironment` was not equal to the previous one -- all due to `Showing` being updated. We now store the canonical `Screen` rendering for each `View` in a tag, and the `Overlay` for each `Dialog` in a tag on its decorView.

Fixes #844
rjrjr added a commit that referenced this issue Jul 29, 2022
Using the `ViewEnvironment` to hold the current `Screen` and `Overlay` was conceptually very clean, but was bad for performance in Compose, which would recompose too often when a new `ViewEnvironment` was not equal to the previous one -- all due to `Showing` being updated. We now store the canonical `Screen` rendering for each `View` in a tag, and the `Overlay` for each `Dialog` in a tag on its decorView.

Fixes #844
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui Related to UI integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant