[Typography] Dynamic Type 2.0 #6733
Merged
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
|
This PR affects multiple components. |
ianegordon
requested review from
jverkoey and
randallli
and removed request for
a team
|
bazel detected changes to the following targets: |
|
This PR affects multiple components. |
|
:/ In this particular case, Kokoro is slightly less than informative.
I propose we merge, barring other comments / concerns / questions. |
|
The reason why it is failing: Let me know if I can help with anything |
|
Strange,... let's take a look tomorrow. |
|
Rerunning kokoro jobs since they seemed to be marked passing this morning but @ianegordon and @yarneo saw failures. |
|
Looks like the test failure was a result of a bad test assertion. It was using |
romoore
pushed a commit
that referenced
this pull request
Mar 7, 2019
Several unit tests were using identity comparison operations (`XCTAssertEqual`) instead of equality comparison (`XCTAssertEqualObjects`). Because UIFont supports `NSCopying`, assigning a UIFont to a `copy` property can result in a copied object. Unit tests seemed to flake sometimes, most recently in a Pull Request for Dynamic Type (#6733). QA=Unit tests pass
|
This PR affects multiple components. |
|
Travis returned one pass and two timeouts. |
jverkoey
approved these changes
Mar 8, 2019
| // If you have queried the table for a sizeCategory that doesn't exist, we will return the | ||
| // traits for XXXL. This handles the case where the values are requested for one of the | ||
| // accessibility size categories beyond XXXL such as | ||
| // UIContentSizeCategoryAccessibilityExtraLarge. Accessbility size categories are only |
There was a problem hiding this comment.
This comment + comment hints at some brittleness here; e.g. what if a custom scaling curve value is missing?
If there is no font size defined for a given scaling curve I'd expect the font not to be adjusted at all; is it possible for us to go with a behavior like that and delete this comment + subsequent code?
if (fontSizeNumber == nil) {
fontSizeNumber = self.mdc_scalingCurve[UIContentSizeCategoryExtraExtraExtraLarge];
}
There was a problem hiding this comment.
A scaling curve should be complete. By complete I mean containing values for all sizes from XXS to XXXL. The accessibility values are optional, which is why there is the fallback behavior of returning XXXL. This matches Apple's previous behavior of only scaling body past XXXL. I'd be happy to change the definition of complete to mean containing values for all sizes from XXS - AXXXL. (Standard Size categories & A11y extensions) This would remove the need for fallback behavior.
I think it is acceptable to throw an exception if a curve is missing a size category.
Thoughts?
There was a problem hiding this comment.
Can we enforce this by creating an object with nonnull properties for each of the expected values, rather than a dictionary? I think this would simultaneously make the code less brittle while also allowing us to encourage good practice when using these APIs.
In other words, instead of using a NSDictionary<NSString *, NSNumber *> as the internal storage, turn that into an object that can be used to store well-typed nonnull values.
There was a problem hiding this comment.
To be (overly?) specific the key in the dictionary is a UIContentSizeCategory, which admittedly is an NSString * :) But NSDictionary<UIContentSizeCategory, NSNumber *> has a lot more context than a vanilla NSString * key.
At some point we are going to have to convert from a UIContentSizeCategory to a CGFloat. If history indicates we are the only people who will be making these curves. I'm confident that an NSDictionary is as simple as possible and will be easily understood by future devs. We do give up some safety, but I think the balance is right.
added 2 commits
March 8, 2019 15:01
…-components-ios into feature-dt2
|
This PR affects multiple components. |
|
This PR affects multiple components. |
|
Merging w/o Travis +1 since it has been timing out recently. |
|
@ianegordon You don't need to wait for Travis-CI or any non-Required checks. Some of those are there because they can be helpful or for testing. |
ianegordon
pushed a commit
that referenced
this pull request
Mar 12, 2019
This reverts commit 648f249.
jverkoey
pushed a commit
that referenced
this pull request
Mar 12, 2019
jverkoey
pushed a commit
that referenced
this pull request
Mar 12, 2019
jverkoey
added a commit
that referenced
this pull request
Mar 13, 2019
Prior to this change, `./scripts/release notes` would consider commits that affected our snapshot goldens (found in `snapshot_test_goldens` as multi-component changes. After this change, snapshot tests that change our goldens are properly bucketed in the relevant component. ## Before ``` ## Changes ### ActionSheet * [Fixes broken action sheet tests. (#6743)](fbcf524) (Robert Moore) ### BottomNavigation * [Clean up internal constants (#6834)](585a5f4) (Robert Moore) * [Examples clean-up. (#6718)](10ed618) (Robert Moore) * [Prevents the client from setting the navigation items directly when using the bottom navigation bar controller. (#6773)](d049bc8) (Eric Lee) ### Buttons * [Allow setting `accessibilityTraits`. (#6766)](66c4664) (Robert Moore) ### Collections * [Clean up interface by removing NS_REQUIRES_SUPER (#6788)](921ad6e) (dmaclach) ### Ripple * [Added a fix for the ripple sometimes blinking when ending animation (#6792)](6f51d26) (Yarden Eitan) * [update docs (#6772)](c2e912d) (Yarden Eitan) ### Snackbar * [Make initialization threadsafe. (#6768)](530c7b9) (Robert Moore) ### Tabs * [Fix badge text truncation bug in MDCItemBarCell (#6786)](188f05a) (Andrew Overton) ## Multi-component changes * [Add basic Snapshot tests (#6826)](2e5df05) (Robert Moore) * [Add basic Snapshot tests. (#6801)](dd363f1) (Robert Moore) * [Add basic Snapshot tests. (#6821)](b1cab54) (Robert Moore) * [Add basic Snapshot tests. (#6823)](1d058b1) (Robert Moore) * [Add basic Snapshot tests. (#6824)](951cc57) (Robert Moore) * [Add basic Snapshot tests. (#6825)](018e72e) (Robert Moore) * [Add basic Snapshot tests. (#6828)](3290460) (Robert Moore) * [Add basic snapshot tests. (#6822)](9e8fdc2) (Robert Moore) * [Dynamic Type 2.0 (#6733)](648f249) (ianegordon) * [Enable -Wunguarded-availability. (#6776)](f17b01c) (featherless) * [Revert "[Typography] Dynamic Type 2.0 (#6733)" (#6848)](7f52f35) (ianegordon) * [{Tests} Fix font comparison in Objective-C. (#6789)](3447c7b) (Robert Moore) ``` ## After ``` ## Changes ### ActionSheet * [Fixes broken action sheet tests. (#6743)](fbcf524) (Robert Moore) ### ActivityIndicator * [Add basic Snapshot tests (#6826)](2e5df05) (Robert Moore) ### BottomNavigation * [Clean up internal constants (#6834)](585a5f4) (Robert Moore) * [Examples clean-up. (#6718)](10ed618) (Robert Moore) * [Prevents the client from setting the navigation items directly when using the bottom navigation bar controller. (#6773)](d049bc8) (Eric Lee) ### Buttons * [Allow setting `accessibilityTraits`. (#6766)](66c4664) (Robert Moore) ### Collections * [Clean up interface by removing NS_REQUIRES_SUPER (#6788)](921ad6e) (dmaclach) ### List * [Add basic snapshot tests. (#6822)](9e8fdc2) (Robert Moore) ### NavigationBar * [Add basic Snapshot tests. (#6821)](b1cab54) (Robert Moore) ### PageControl * [Add basic Snapshot tests. (#6823)](1d058b1) (Robert Moore) ### ProgressView * [Add basic Snapshot tests. (#6825)](018e72e) (Robert Moore) ### Ripple * [Added a fix for the ripple sometimes blinking when ending animation (#6792)](6f51d26) (Yarden Eitan) * [update docs (#6772)](c2e912d) (Yarden Eitan) ### Snackbar * [Add basic Snapshot tests. (#6824)](951cc57) (Robert Moore) * [Make initialization threadsafe. (#6768)](530c7b9) (Robert Moore) ### Tabs * [Add basic Snapshot tests. (#6801)](dd363f1) (Robert Moore) * [Fix badge text truncation bug in MDCItemBarCell (#6786)](188f05a) (Andrew Overton) ### Typography * [Add basic Snapshot tests. (#6828)](3290460) (Robert Moore) ## Multi-component changes * [Dynamic Type 2.0 (#6733)](648f249) (ianegordon) * [Enable -Wunguarded-availability. (#6776)](f17b01c) (featherless) * [Revert "[Typography] Dynamic Type 2.0 (#6733)" (#6848)](7f52f35) (ianegordon) * [{Tests} Fix font comparison in Objective-C. (#6789)](3447c7b) (Robert Moore) ```
jverkoey
added a commit
that referenced
this pull request
Mar 13, 2019
Prior to this change, `./scripts/release notes` would consider commits that affected our snapshot goldens (found in `snapshot_test_goldens` as multi-component changes. After this change, snapshot tests that change our goldens are properly bucketed in the relevant component. ## Before ``` ## Changes ### ActionSheet * [Fixes broken action sheet tests. (#6743)](fbcf524) (Robert Moore) ### BottomNavigation * [Clean up internal constants (#6834)](585a5f4) (Robert Moore) * [Examples clean-up. (#6718)](10ed618) (Robert Moore) * [Prevents the client from setting the navigation items directly when using the bottom navigation bar controller. (#6773)](d049bc8) (Eric Lee) ### Buttons * [Allow setting `accessibilityTraits`. (#6766)](66c4664) (Robert Moore) ### Collections * [Clean up interface by removing NS_REQUIRES_SUPER (#6788)](921ad6e) (dmaclach) ### Ripple * [Added a fix for the ripple sometimes blinking when ending animation (#6792)](6f51d26) (Yarden Eitan) * [update docs (#6772)](c2e912d) (Yarden Eitan) ### Snackbar * [Make initialization threadsafe. (#6768)](530c7b9) (Robert Moore) ### Tabs * [Fix badge text truncation bug in MDCItemBarCell (#6786)](188f05a) (Andrew Overton) ## Multi-component changes * [Add basic Snapshot tests (#6826)](2e5df05) (Robert Moore) * [Add basic Snapshot tests. (#6801)](dd363f1) (Robert Moore) * [Add basic Snapshot tests. (#6821)](b1cab54) (Robert Moore) * [Add basic Snapshot tests. (#6823)](1d058b1) (Robert Moore) * [Add basic Snapshot tests. (#6824)](951cc57) (Robert Moore) * [Add basic Snapshot tests. (#6825)](018e72e) (Robert Moore) * [Add basic Snapshot tests. (#6828)](3290460) (Robert Moore) * [Add basic snapshot tests. (#6822)](9e8fdc2) (Robert Moore) * [Dynamic Type 2.0 (#6733)](648f249) (ianegordon) * [Enable -Wunguarded-availability. (#6776)](f17b01c) (featherless) * [Revert "[Typography] Dynamic Type 2.0 (#6733)" (#6848)](7f52f35) (ianegordon) * [{Tests} Fix font comparison in Objective-C. (#6789)](3447c7b) (Robert Moore) ``` ## After ``` ## Changes ### ActionSheet * [Fixes broken action sheet tests. (#6743)](fbcf524) (Robert Moore) ### ActivityIndicator * [Add basic Snapshot tests (#6826)](2e5df05) (Robert Moore) ### BottomNavigation * [Clean up internal constants (#6834)](585a5f4) (Robert Moore) * [Examples clean-up. (#6718)](10ed618) (Robert Moore) * [Prevents the client from setting the navigation items directly when using the bottom navigation bar controller. (#6773)](d049bc8) (Eric Lee) ### Buttons * [Allow setting `accessibilityTraits`. (#6766)](66c4664) (Robert Moore) ### Collections * [Clean up interface by removing NS_REQUIRES_SUPER (#6788)](921ad6e) (dmaclach) ### List * [Add basic snapshot tests. (#6822)](9e8fdc2) (Robert Moore) ### NavigationBar * [Add basic Snapshot tests. (#6821)](b1cab54) (Robert Moore) ### PageControl * [Add basic Snapshot tests. (#6823)](1d058b1) (Robert Moore) ### ProgressView * [Add basic Snapshot tests. (#6825)](018e72e) (Robert Moore) ### Ripple * [Added a fix for the ripple sometimes blinking when ending animation (#6792)](6f51d26) (Yarden Eitan) * [update docs (#6772)](c2e912d) (Yarden Eitan) ### Snackbar * [Add basic Snapshot tests. (#6824)](951cc57) (Robert Moore) * [Make initialization threadsafe. (#6768)](530c7b9) (Robert Moore) ### Tabs * [Add basic Snapshot tests. (#6801)](dd363f1) (Robert Moore) * [Fix badge text truncation bug in MDCItemBarCell (#6786)](188f05a) (Andrew Overton) ### Typography * [Add basic Snapshot tests. (#6828)](3290460) (Robert Moore) ## Multi-component changes * [Dynamic Type 2.0 (#6733)](648f249) (ianegordon) * [Enable -Wunguarded-availability. (#6776)](f17b01c) (featherless) * [Revert "[Typography] Dynamic Type 2.0 (#6733)" (#6848)](7f52f35) (ianegordon) * [{Tests} Fix font comparison in Objective-C. (#6789)](3447c7b) (Robert Moore) ```
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.
Next iteration of our Dynamic Type.
Mimics Apple's new APIs. (UIFontMetrics)
Adds MDCFontScalar to attach curves.
Adds UIFont category to expose fontScaledForCategory methods.
Adds new MDCTypographyScheme with scalable fonts.