✨ Relax the iOS fixed-position transfer requirements #16049
Conversation
|
Pinging @choumx for next steps here and assigning it to them as well. |
jridgewell
force-pushed
the
loosen-ios-fixed-transfer
branch
from
4715f10
to
9bb8a3f
Compare
| opacity > 0 && | ||
| offsetHeight < 300 && | ||
| (this.isAllowedCoord_(top) || this.isAllowedCoord_(bottom)))); | ||
| fe.forceTransfer || (opacity > 0 && !!(top || bottom))); |
There was a problem hiding this comment.
This will return true if top: 1 and bottom: 1, right? I think you want (!top || !bottom).
There was a problem hiding this comment.
This matches current behavior. You could spec top: 0; bottom: 0 which passes the old isAllowedCoord_.
There was a problem hiding this comment.
Oh they're strings not numbers.
Ok, for posterity: we only transfer fixed elements that are not auto-positioned to avoid jumping position after transferring to the fixed layer (due to loss of parent positioning context). We could do this work, but we don't (yet).
| // transparent) - that's a lot of work for no benefit. Additionally, | ||
| // transparent elements used for "service" needs and thus best kept | ||
| // in the original tree. The visibility, however, is not considered | ||
| // because `visibility` CSS is inherited. |
There was a problem hiding this comment.
Nit: Add "We also only consider elements stuck to the top or bottom edge of the viewport as an optimization heuristic."
There was a problem hiding this comment.
We're dropping the top/bottom requirement. 😛
jridgewell
force-pushed
the
loosen-ios-fixed-transfer
branch
from
9bb8a3f
to
a4d8f52
Compare
| opacity > 0 && | ||
| offsetHeight < 300 && | ||
| (this.isAllowedCoord_(top) || this.isAllowedCoord_(bottom)))); | ||
| fe.forceTransfer || (opacity > 0 && !!(top || bottom))); |
There was a problem hiding this comment.
Oh they're strings not numbers.
Ok, for posterity: we only transfer fixed elements that are not auto-positioned to avoid jumping position after transferring to the fixed layer (due to loss of parent positioning context). We could do this work, but we don't (yet).
Removes the max element height restriction, and allows any declared top or bottom to be transfered.
jridgewell
force-pushed
the
loosen-ios-fixed-transfer
branch
from
a4d8f52
to
da001db
Compare
* Relax the iOS fixed-position transfer requirements Removes the max element height restriction, and allows any declared top or bottom to be transfered. * Fix tests
Partially reverts ampproject#16049. Fixes ampproject#17995.
Partially reverts ampproject#16049. Fixes ampproject#17995.
…8578) Partially reverts ampproject#16049. Fixes ampproject#17995.
* Relax the iOS fixed-position transfer requirements Removes the max element height restriction, and allows any declared top or bottom to be transfered. * Fix tests
…8578) Partially reverts ampproject#16049. Fixes ampproject#17995.
Removes the max element height restriction, and allows any declared top or bottom to be transferred.
Fixes #15809.
Starts work on #14788.