Replaces Showing and InOverlay with View tags.
#845
Merged
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
rjrjr
force-pushed
the
ray/let-it-show-let-it-show-let-it-show
branch
2 times, most recently
from
f9ebc90 to
7b5d2f5
Compare
rjrjr
force-pushed
the
ray/back-button-always-the-back-button-what-a-terrible-idea-the-back-button-was
branch
2 times, most recently
from
2120968 to
ba43dab
Compare
rjrjr
force-pushed
the
ray/let-it-show-let-it-show-let-it-show
branch
2 times, most recently
from
e57f231 to
f6c0181
Compare
Base automatically changed from
ray/back-button-always-the-back-button-what-a-terrible-idea-the-back-button-was
to
main
July 28, 2022 23:39
0legg
approved these changes
Jul 29, 2022
| @WorkflowUiExperimentalApi | ||
| public var View.screen: Screen | ||
| get() = checkNotNull(screenOrNull) { | ||
| "Expected to find a Screen in tag workflow_screen (${R.id.workflow_screen})" |
There was a problem hiding this comment.
${R.id.workflow_screen} will render to some integer that won't make sense to users.
There was a problem hiding this comment.
Yeah, I guess it won't be useful. I was figuring that by providing the name as well I was giving a hint that they could look for the magic number on other views or something.
| @WorkflowUiExperimentalApi | ||
| public var Dialog.overlay: Overlay | ||
| get() = checkNotNull(overlayOrNull) { | ||
| "Expected to find an Overlay in tag workflow_overlay (${R.id.workflow_overlay}) " + |
There was a problem hiding this comment.
The same thing about integer value of the tag.
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
force-pushed
the
ray/let-it-show-let-it-show-let-it-show
branch
from
f6c0181 to
98ebcbd
Compare
RBusarow
reviewed
Jul 29, 2022
|
|
||
| /** | ||
| * Returns the most recent [Overlay] rendering [shown][OverlayDialogHolder.show] in this [Dialog], | ||
| * or throws a [NullPointerException] ` if the receiver was not created via |
There was a problem hiding this comment.
Suggested change
| * or throws a [NullPointerException] ` if the receiver was not created via | |
| * or throws a [NullPointerException] if the receiver was not created via |
RBusarow
approved these changes
Jul 29, 2022
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.
Using the
ViewEnvironmentto hold the currentScreenandOverlaywas conceptually very clean, but was bad for performance in Compose, which would recompose too often when a newViewEnvironmentwas not equal to the previous one -- all due toShowingbeing updated. We now store the canonicalScreenrendering for eachViewin a tag, and theOverlayfor eachDialogin a tag on its decorView.Fixes #844