Adding safe area edge detection for the scrollableAxes behavior#596
Conversation
b4f0481 to
f3e9924
Compare
f3e9924 to
3b59837
Compare
…Label (#597) This reverts #590. During integration testing, the original change caused KIF failures while searching for labels with the `staticText` trait. The work to audit the failures and make necessary code changes has been ticketed [here](https://block.atlassian.net/browse/UI-9567). This revert allows the changes in #596 to be fully tested.
* main: Reverting the change to remove accessibility traits on the AttributedLabel (#597)
- Renaming processScrollableAxesEdgeConfigurations. - Hoisting calculateConfiguration to the containing class and renaming its closure.
watt
left a comment
There was a problem hiding this comment.
Very thorough and well-documented, nice work!
|
|
||
| /// Determines whether `scrollableAxesSafeAreaEdges` has been modified by the | ||
| /// client or not. | ||
| public private(set) var didModifyScrollableAxesSafeAreaEdges: Bool = false |
There was a problem hiding this comment.
Is this meant to be used somewhere, or just cruft?
There was a problem hiding this comment.
This property is used to minimally apply the .bottom edge if the client hasn’t already set an edge, in this PR. If the client already configured the edge behavior, this property is used to avoid making further changes.
There was a problem hiding this comment.
After chatting in person, we discussed defaulting scrollableAxesSafeAreaEdges to nil, which we could then check against to determine if the client has modified the property.
(In the linked PR, we can ask those clients to supply the empty set if they want to opt out of the .bottom inset behavior.)
- Remove
didModifyScrollableAxesSafeAreaEdgesand makescrollableAxesSafeAreaEdgesdefault tonil.
| /// the safe area. | ||
| /// | ||
| /// The default value is the empty set, which leverages standard UIKit behavior. | ||
| public var scrollableAxesSafeAreaEdges: SafeAreaEdge = [] { |
There was a problem hiding this comment.
Is there any reason not to make the default .all?
There was a problem hiding this comment.
Thanks for the suggestion. There were a few reasons that the empty set was used for the default:
- Specifying
SafeAreaEdge.allwill have the same effect as setting thecontentInsetAdjustmentBehaviorto.always. - Including the
.topedge breaks a number of snapshots that currently draw content within the top safe area. Similar to the snapshot breakages,.topcan change expected behavior at runtime if a client aligned the ScrollView to the screen's top edge and manually adjusted its content to avoid the safe area by including padding around their elements. - Including the horizontal edges can also break snapshots and will enable horizontal scrolling in contexts that traditionally have not horizontally scrolled, like cases where we’re using
ContentSize.fittingHeightand constraining the ScrollView to the screen's left or right edge.
I'd like to provide this control knob to clients without enabling it automatically, with the exception of this change, where we’re minimally enabling the .bottom edge. Happy to chat further though!
I can make .all internal and add a note explaining that clients should use ContentInsetAdjustmentBehavior.always instead:
- Update
SafeAreaEdge.all
- Using an autoclosure. - Moving ScrollViewUITests into BlueprintUI-Tests and renaming. - Making SafeAreaEdge.all internal. - Changing ContentEdgeConfigurations.none to a constant.
Removing didModifyScrollableAxesSafeAreaEdges.
## What's Changed * Reverting the change to remove accessibility traits on the AttributedLabel by @johnnewman-square in #597 * Adding safe area edge detection for the scrollableAxes behavior by @johnnewman-square in #596 **Full Changelog**: 6.3.1...6.4.0
* main: chore: Release 6.5.0 (#600) refactor: Migrate Accessibility Infrastructure from Market Design System (#593) chore: Release 6.4.0 (#599) Adding safe area edge detection for the scrollableAxes behavior (#596) Reverting the change to remove accessibility traits on the AttributedLabel (#597) Update changelog for 6.3.1 release (#591) Fix warnings in Xcode 26 (#589) Apply empty accessibility traits to AttributedLabel if supplied traits is nil Bump rexml from 3.3.9 to 3.4.2
* maxg/cache_1_equivalency: chore: Release 6.5.0 (#600) refactor: Migrate Accessibility Infrastructure from Market Design System (#593) chore: Release 6.4.0 (#599) Adding safe area edge detection for the scrollableAxes behavior (#596) Reverting the change to remove accessibility traits on the AttributedLabel (#597) Update changelog for 6.3.1 release (#591) Fix warnings in Xcode 26 (#589) Apply empty accessibility traits to AttributedLabel if supplied traits is nil Bump rexml from 3.3.9 to 3.4.2
Adding a
ScrollViewAPI to allow clients to specify the edges that should be inset for the safe area when using thescrollableAxescontent inset behavior. Before this change, content that landed outside the safe area but inside the scroll view bounds was not scrollable when usingscrollableAxes. (See linked docs for details)The goal of this update is to address the issue of content not being scrollable without introducing breaking API changes or large behavioral changes to ScrollView. A few alternative approaches could be:
alwaysBouncesVerticaland/oralwaysBounceHorizontal, which will causescrollableAxesto honor the safe areas along both edges of the bouncing axes.alwayscontent inset behavior. This would require refactoring the ScrollView layout to no longer produce acontentSizeequal to the ScrollView bounds when usingfittingWidthandfittingHeight.The approach in this PR will enable the safe area behavior we need along a precise edge, like the bottom, while limiting the risk of regressions.