Fix app-breaking card-position regression from always-on floatingCardToolBarItems constraints#30
Merged
Conversation
…nstraints `floatingCardToolBarItems` (2.5.0) added `cardFloatingView` to the controller's root view in `viewDidLoad` and *permanently* pinned it to `cardWrapperContent` — the card's content wrapper — including a bottom constraint to the wrapper's bottom edge. Those constraints were active for **every** card, even ones that provide no floating items. Because the wrapper animates (its top/height constraints change) on push/pop, coupling a root-level view to it via that bottom constraint disrupted the card's position layout: after pushing then popping a card, the wrapper could be left visually extended while `cardWrapperDesiredTopConstraint` (and therefore `cardPosition`) read collapsed — so the content faded out, the map buttons showed, and the card couldn't be dragged. It reproduced on plain cards (e.g. a timetable card) that don't use the affordance at all. Two changes: - Install `cardFloatingView` and its constraints lazily — only while a card actually has `floatingCardToolBarItems` — and remove them again otherwise. Cards without items now carry none of this layout, exactly as before 2.5.0. - Pin the bottom to the screen's safe area instead of `cardWrapperContent`. The card always reaches at or below the screen edge, so it's the same place visually, but it no longer couples to the wrapper as it animates. Horizontal alignment stays relative to the card (its x/width don't move on a vertical transition). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
Author
|
Suggested squash-merge commit message: After merge: tag 2.5.1, then bump TripGo's TGCardViewController pin from 2.5.0 to 2.5.1 (2.5.0 ships this regression; it currently blocks the agenda umbrella PR tripgo-ios#101). |
This file contains hidden or 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
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.
Fixes an app-breaking card-position regression in 2.5.0
floatingCardToolBarItems(2.5.0, #29) addedcardFloatingViewto the controller's root view inviewDidLoadand permanently pinned it tocardWrapperContent— the card's content wrapper — including abottom == cardWrapperContent.bottomconstraint. Those constraints were active for every card, even ones that provide no floating items.Because the wrapper animates (its top/height constraints change) on push/pop, coupling a root-level view to it via that bottom constraint disrupted the card's position layout. After pushing then popping a card, the wrapper was left visually extended while
cardWrapperDesiredTopConstraint— and therefore the computedcardPosition— read collapsed. Result: content faded out, map toolbar buttons showed (the map becomes interactive when not extended), and the card couldn't be dragged. It reproduced on plain cards that don't use the affordance at all (e.g. tap a stop → timetable card pushes → pop → wedged; needs a force-quit).Fix
cardFloatingViewand its constraints are added only while a card actually hasfloatingCardToolBarItems, and torn down again otherwise. Cards without items now carry none of this layout — exactly as before 2.5.0. This alone resolves the regression for non-adopting cards.cardWrapperContent.bottom. The card always reaches at or below the screen edge, so it's the same place visually, but it no longer couples to the wrapper as it animates — protecting adopting cards' own transitions too. Horizontal alignment stays relative to the card (that axis doesn't move during a vertical transition); the leading/trailing inequality clamps still keep a wide row from overflowing.The agenda pill looks and behaves identically (the old "prefer card bottom, clamped to safe area" always resolved to the safe-area clamp anyway, since the card always reaches the screen edge).
Reviewer notes
floatingCardToolBarItems/floatingCardToolBarAlignmentare unchanged; only the internal install/teardown and the bottom-pin target changed.🤖 Generated with Claude Code