-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[Woo POS] Improve Checkout view adaptability to large font sizes #15153
[Woo POS] Improve Checkout view adaptability to large font sizes #15153
Conversation
…izeCategory POSFontStyle should react to global dynamicTypeSize changes to make it more versatile
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Experience is much better now for larger accessibility sizes 🚢
Updated payment state views to either hide images or shrink images when accessibility increased
Yes, IMHO this is something we don't do often enough 👏
I've logged several accessibility-related issues I've found while testing, but unrelated to your changes:
private var bannerHintAndLearnMoreText: some View { | ||
Text("\(headerBannerHint) \(Localization.headerBannerLearnMoreHint)") | ||
.font(.posBodySmallBold) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use something like String.localizedStringWithFormat( ... )
here rather than string concatenation? For localization in case of sites/app settings running RTL languages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. It also depends how we want to present this part. On Android, only "Learn more" is presented in primary color while on iOS we show these two sentences from a new line. Either way, "Learn more" looks to be designed to be shown in a new sentence.
Closes: #15123
Before tackling #15152, I'm updating the view to better support larger accessibility sizes
Description
The idea was to play with
layoutPriority
. However, it only works up to the point, we need to give permission for views to shrink, either by making resizable images (.resizable
) or resizable texts (.minimumScaleFactor
). Also, TotalsView texts are large enough for non-scroll view, so I decided to limit the size withdynamicTypeSize
range.Changes:
POSFontStyle
to supportdynamicTypeSize
range, replaced existingUIContentSize
usageSteps to reproduce
I suggest testing with:
Test these checkout states with different accessibility sizes:
Testing information
Tested with iPad Air M2 and iPad Mini A17
Screenshots
iPad.mini.A17.Pro.Accessibility.mp4
RELEASE-NOTES.txt
if necessary.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: