-
Notifications
You must be signed in to change notification settings - Fork 115
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
Enable scrolling in card present modals when it cannot grow in size anymore #15328
Conversation
The modal continues to behave as before - grows in size if content expands but when the content doesn't fit anymore the modal starts support scrolling. In 99% of cases it only kicks in with larger dynamic type size
Only applies to large accessibility sizes and long button titles
Modals now support scrolling so longer content can be shown without cutting it off or shrinking it
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request updates the font styling for email address text in two view model classes by replacing a fixed bold system font with a dynamic preferred font for body text. Additionally, it adjusts UI properties in a modal view controller and its associated XIB file—setting labels to allow unlimited lines, reducing button title scale factors, and restructuring the layout from a vertical stack view to a scroll view with updated constraints and outlet references. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
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.
Thanks for improving this! Works well.
Closes: #15321
Description
CardPresentPaymentsModalViewController
used for all the modals during the card reader connection and payment process may cut some content if it doesn't fit anymore.CardPresentPaymentsModalViewController
already has dynamic abilities, since it grows from minimum to maximum size while the content is growing. However, if modals contain a lot of elements with larger dynamic type sizes, then some content may be cut.Solution
Wrap modal content in the
UISCrollView
. I had to play around with constraints and priorities to make sure scrolling only kicked in once the modal could not grow in size anymore.Due to the addition of scroll view, I removed limits of how long the text can be, so it wouldn't be cut or shrunk when dynamic type size grows.
Additionally, there were issues with button content not fitting. I couldn't quickly figure out why buttons cannot go into multiline so decided to reduce
minimumScaleFactor
. This is not a perfect solution but I don't notice issues with button text being cut-off anymore.Possible improvements
There could be more improvements to this solution. For example, growing modal in width, especially on iPad, once a certain accessibility size is reached. We could also put a button container in a separate scroll view. It looks like this is what native iOS alerts do, so both some actions and some content are always visible.
Steps to reproduce
Ask next time
Testing information
The constraints differ depending on the size class, so I performed testing both at Storyboard level, iPhone 14 Pro 17.7 device, iPad Air M2 18.3 device, and simulator to make sure the changes in layout didn't break anything.
Screenshots
iPad Device
iPad.-.Card.Present.Modal.Self.Sizing.Scroll.View.MP4
iPhone Device
iPhone.-.Card.Present.Payment.Scrolling.View.mov
iPad Simulator
iPad.-.Dynamic.Type.Size.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: