Financial Connections: adjusted typography and spacing for Institution Picker #2543
Conversation
This file contains 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
kgaidis-stripe
force-pushed
the
fc-fontspacing
branch
from
adb1003
to
2068dc8
Compare
kgaidis-stripe
force-pushed
the
kg-fontinstitution
branch
from
685ec8e
to
c26f719
Compare
…abelNew to AttributedTextView.
…vertical spacing and fixed more institution references to use the new font infra.
…w to be more robust.
kgaidis-stripe
force-pushed
the
kg-fontinstitution
branch
from
717699f
to
5ee6991
Compare
…e new font infra.
kgaidis-stripe
commented
May 9, 2023
| @@ -1,156 +0,0 @@ | |||
| // | |||
| // InstitutionSearchErrorView.swift | |||
There was a problem hiding this comment.
This was dead code - you should see no references to this anywhere but here
kgaidis-stripe
force-pushed
the
kg-fontinstitution
branch
from
78238ae
to
aebd031
Compare
kgaidis-stripe
commented
May 9, 2023
Comment on lines
+27
to
+28
| // UILabel with custom `lineHeight` via `NSParagraphStyle` was not properly | ||
| // centering the text, so here we adjust it to be centered. |
There was a problem hiding this comment.
this explains why I introduced AttributedLabel...the test plan shows the difference too
There was a problem hiding this comment.
Well this comment + the fact that ClickableLabelNew (now AttributedTextView) does not support single line text (at least easily)
kgaidis-stripe
commented
May 9, 2023
| @@ -11,7 +11,10 @@ import SafariServices | |||
| @_spi(STP) import StripeUICore | |||
| import UIKit | |||
|
|
|||
| final class ClickableLabelNew: HitTestView { | |||
There was a problem hiding this comment.
Renamed because ClickableLabelNew made me confused: I tried to use it as a UILabel replacement, but UITextView is multi-line, so its not a direct replacement. AttributedTextView also matches with its 'similar' AttributedLabel.
kgaidis-stripe
commented
May 9, 2023
Comment on lines
+188
to
+189
| // UITextView with custom `lineHeight` via `NSParagraphStyle` was not properly | ||
| // centering the text, so here we adjust it to be centered. |
There was a problem hiding this comment.
test plan shows the difference of what this centering-logic achieved
vardges-stripe
approved these changes
May 10, 2023
| let paragraphStyleLineHeight = paragraphStyle.minimumLineHeight | ||
| assert(paragraphStyle.minimumLineHeight == paragraphStyle.maximumLineHeight, "we are assuming that minimum and maximum are the same") | ||
|
|
||
| if paragraphStyleLineHeight > uiFontLineHeight { |
kgaidis-stripe
added a commit
that referenced
this pull request
May 15, 2023
…n Picker (#2543) * Financial Connections: adjusted fonts for Institution Picker. * Financial Connections: adjusted fonts for prepane. * Financial Connections: created AttributedLabel and renamed ClickableLabelNew to AttributedTextView. * Financial Connections: modified AttributedTextView to support better vertical spacing and fixed more institution references to use the new font infra. * Financial Connections: modified AttributedLabel and AttributedTextView to be more robust. * Financial Connections: changed FeaturedInstitutionGridCell font to use new font infra. * Financial Connections: small font fix.
kgaidis-stripe
added a commit
that referenced
this pull request
May 17, 2023
…n Picker (#2543) * Financial Connections: adjusted fonts for Institution Picker. * Financial Connections: adjusted fonts for prepane. * Financial Connections: created AttributedLabel and renamed ClickableLabelNew to AttributedTextView. * Financial Connections: modified AttributedTextView to support better vertical spacing and fixed more institution references to use the new font infra. * Financial Connections: modified AttributedLabel and AttributedTextView to be more robust. * Financial Connections: changed FeaturedInstitutionGridCell font to use new font infra. * Financial Connections: small font fix.
kgaidis-stripe
added a commit
that referenced
this pull request
May 23, 2023
* Financial Connections: introduced FinancialConnectionsFont and ClickableLabelNew to support line spacing. * Financial Connections: modified the consent bottom sheet fonts/spacing. * Financial Connections: adjusted typography and spacing for Institution Picker (#2543) * Financial Connections: adjusted fonts for Institution Picker. * Financial Connections: adjusted fonts for prepane. * Financial Connections: created AttributedLabel and renamed ClickableLabelNew to AttributedTextView. * Financial Connections: modified AttributedTextView to support better vertical spacing and fixed more institution references to use the new font infra. * Financial Connections: modified AttributedLabel and AttributedTextView to be more robust. * Financial Connections: changed FeaturedInstitutionGridCell font to use new font infra. * Financial Connections: small font fix. * Financial Connections: changed account picker typography. * Financial Connections: changed Networking Account Picker typography. * Financial Connections: changed success pane typography * Financial Connections: changed networking sign up pane typography. * Financial Connections: modified warmup pane typography. * Financial Connections: changed the typography in networking verification panes. * Financial Connections: more changes to adopt to new typography. * Financial Connections: fixed search bar spacing for testmode. * Financial Connections: fixed unit tests having old font references.
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.
Summary
^ see title, and I am leaving various comments in this PR to explain specific parts
Design and font references:
https://www.figma.com/file/ikSIQS9Lw0qIr3PSeyyrPP/Connections-V2.5?type=design&node-id=1636-89037&mode=design&t=vw8IsvjVsgHt67RL-0
https://sail.stripe.me/foundations/typography/
https://www.figma.com/file/WAFvoybps5uk3FL4tftaee/Sail?type=design&node-id=26-370&mode=design&t=Hd2oiFSiURfCMlmn-0
Testing
UILabel font adjustment to be centered
UITextView font adjustment to be centered
Various Screens That Were Adjusted Due To The Changes