-
-
Notifications
You must be signed in to change notification settings - Fork 746
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
Define type styles in WKFont, update project to use only WKFont #4866
Conversation
…f the UIFont extension
…nstead of the UIFont extension
…nstead of the UIFont extension, adds some cleanup comments for future commits
…o components-fonts-update
…o components-fonts-update
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.
@mazevedofs Great stuff! Some comments after an initial (very quick) code-only review. I will dig deeper in the near future.
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.
I found a few more comments after a slower code review.
ContinueReadingWidget/WMFTodayContinueReadingWidgetViewController.swift
Outdated
Show resolved
Hide resolved
WMF Framework/Remote Notifications/RemoteNotificationsAPIController.swift
Outdated
Show resolved
Hide resolved
@mazevedofs FYI, I also pushed an bug fix of something existing that's been bothering me. For Image Recommendations and Watchlist, our SwiftUI views do not update the font size upon foreground after changing dynamic type in Settings. This commit fixes it. |
I notice the font has changed enough on this widget to cause a lot of additional spacing at the bottom: |
@tonisevener This widget used SwiftUI caption (the same as caption1 on UIKit), so if there's a significant difference between them, we might need to adjust the Widget. |
Hmm yeah that is strange, I'm not sure why the difference is occurring. What's interesting is that all font sizes seem to have shrunk for me. I'm testing on my iPhone 12 Pro Max. |
Per discussion: We'll adjust widgets after another round of design review as-needed. |
…o components-fonts-update
Phabricator: https://phabricator.wikimedia.org/T300034
Notes
Since we have less styles now, I swapped some fonts based on approximate size and weight
Test Steps
Heavier smoke testing will be done by the QA