forked from fzyzcjy/flutter_portal
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Portal refactor & Anchor
implementation
#1
Closed
creativecreatorormaybenot
wants to merge
36
commits into
TurtleAI:portal-anchor-refactor
from
creativecreatorormaybenot:portal-anchor-refactor
Closed
Portal refactor & Anchor
implementation
#1
creativecreatorormaybenot
wants to merge
36
commits into
TurtleAI:portal-anchor-refactor
from
creativecreatorormaybenot:portal-anchor-refactor
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
creativecreatorormaybenot
changed the title
Portal anchor refactor
Implement root bounds for portal anchor refactor
Oct 25, 2021
creativecreatorormaybenot
changed the title
Implement root bounds for portal anchor refactor
Portal refactor & Feb 7, 2022
Anchor
implementation
Current state: the new concepts are introduced and applied everywhere. I have touched all of the docs and examples to use the new concepts and it all seems coherent to me. What is still open is handling the off-screen cut-off. |
venkatd
reviewed
Feb 10, 2022
…warning: 'top_level_function_literal_block' isn't a recognized error code. (unrecognized_error_code at [flutter_portal] analysis_options.yaml:16)`
…eld is no longer used and can be removed. (deprecated_field at [flutter_portal] pubspec.yaml:5)`; and doc says `Deprecated. Use a verified publisher instead. ... The pub.dev site no longer displays package authors`
Closing this here as the changes have been merged upstream in fzyzcjy#44. |
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.
Description
portalRect
(renamed fromoverlayRect
) togetFollowerOffset
(renamed fromgetSourceOffset
).Portal
,PortalTarget
,PortalFollower
, andAnchor
.Implementation details
The passing of root bounds is achieved using a custom
FollowerLayer
implementation that takes a callback for computing thelinkedOffset
. This allows making thelinkedOffset
depend on the offset of the leader layer (viaCompositedTransformTarget
). And this in turn allows to make thelinkedOffset
depend on thetheaterRect
, which requires only the relative top-left offset to thetargetRect
and the size of the theater, which we can obtain via the_RenderPortalTheater
render box.Tests
This PR only contains two unit test cases, but I did not feel the need to add more test cases since these are already the cases that cover the "most possible" edge cases, i.e. a portal that is not the root of the app and portal entry sizes that differ from the portal and child sizes.
Fragility
Doing this with TDD really helped me to come to a non-fragile implementation. Initially, I thought it would be enough to call
localToGlobal
withOffset.zero
and the theater render box ancestor. However, this does not work because it does not respect the offset of theLayerLink
. After that, I thought that I could access thelink.leader
inpaint
directly. Using a non-null assertion worked fine in my example app case, however, the unit tests revealed that this was actually not safe to do on the first paint call. This is because the layer link leader only attaches itself after the painting phase. And this is how I finally arrived at creating a customFollowerLayer
, which allows to access the leader layer offset in the composition phase.This is why I believe that the new implementation is non-fragile and solid.
We can of course add more test cases - it cannot hurt :)