-
Notifications
You must be signed in to change notification settings - Fork 5
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
IOS-10491 Update Title component #397
Changes from 4 commits
ece02ce
bae3c36
a5e3dd0
698ee7c
e26fce0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,21 +19,29 @@ public class TitleView: UIView { | |
public enum Style { | ||
case title1 | ||
case title2 | ||
case title3 | ||
case title4 | ||
|
||
var font: UIFont { | ||
switch self { | ||
case .title1: | ||
return .textPreset1(weight: .title1) | ||
case .title2: | ||
return .textPreset5() | ||
return .textPreset3(weight: .title2) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How can we migrate this @aweell ??? Checking this, where we used |
||
case .title3: | ||
return .textTitle3() | ||
case .title4: | ||
return .textPreset6() | ||
} | ||
} | ||
|
||
var textColor: UIColor { | ||
switch self { | ||
case .title1: | ||
return .textSecondary | ||
case .title2: | ||
case .title2, | ||
.title3, | ||
.title4: | ||
return .textPrimary | ||
} | ||
} | ||
|
@@ -42,7 +50,9 @@ public class TitleView: UIView { | |
switch self { | ||
case .title1: | ||
return text?.uppercased() | ||
case .title2: | ||
case .title2, | ||
.title3, | ||
.title4: | ||
return text | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ public extension FontStyle { | |
} | ||
|
||
enum TextPreset3Weight: CaseIterable { | ||
case light, regular, medium, button, link | ||
case light, regular, medium, button, link, title2, title3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a little bit weird. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like we're messing and reusing fonts and weights namings, we should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's due to |
||
|
||
var value: MisticaFontWeightType { | ||
switch self { | ||
|
@@ -49,6 +49,8 @@ public extension FontStyle { | |
case .medium: return .medium | ||
case .button: return MisticaConfig.currentFontWeights.button | ||
case .link: return MisticaConfig.currentFontWeights.link | ||
case .title2: return MisticaConfig.currentFontWeights.title2 | ||
case .title3: return MisticaConfig.currentFontWeights.title3 | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import UIKit | |
case textPreset9 | ||
case textPreset10 | ||
case textPresetTabsLabel | ||
case textTitle3 | ||
|
||
func preferredFont( | ||
weight: Font.Weight, | ||
|
@@ -78,6 +79,8 @@ import UIKit | |
return "TextPreset10" | ||
case .textPresetTabsLabel: | ||
return "TextPresetTabsLabel" | ||
case .textTitle3: | ||
return "TextPresetTitle3" | ||
} | ||
} | ||
} | ||
|
@@ -130,6 +133,8 @@ private extension FontStyle { | |
return 48 | ||
case .textPresetTabsLabel: | ||
return MisticaConfig.currentFontSizes.tabsLabel | ||
case .textTitle3: | ||
return MisticaConfig.currentFontSizes.title3 | ||
} | ||
} | ||
|
||
|
@@ -144,7 +149,8 @@ private extension FontStyle { | |
return .body | ||
case .textPreset4: | ||
return .headline | ||
case .textPreset5: | ||
case .textPreset5, | ||
.textTitle3: | ||
Comment on lines
+152
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've considered |
||
return .title3 | ||
case .textPreset6: | ||
return .title2 | ||
|
@@ -168,7 +174,8 @@ private extension FontStyle { | |
return .body | ||
case .textPreset4: | ||
return .headline | ||
case .textPreset5: | ||
case .textPreset5, | ||
.textTitle3: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've considered |
||
return .title3 | ||
case .textPreset6: | ||
return .title2 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,78 @@ final class TitleHeaderFooterViewTests: XCTestCase { | |
viewBuilder: makeSectionTitle(title: "This is a very long test text to check multiline", linkTitle: "Link text", style: .title2) | ||
) | ||
} | ||
|
||
func testTitle3() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. title 3 uses different presets depending on the brand so recording this for two brands is enough IMO |
||
assertSnapshot( | ||
for: [.vivoNew, .movistar], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "Title text", style: .title3) | ||
) | ||
} | ||
|
||
func testTitle3WithLink() { | ||
assertSnapshot( | ||
for: [.vivoNew, .movistar], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "Title text", linkTitle: "Link text", style: .title3) | ||
) | ||
} | ||
|
||
func testTitle3Multiline() { | ||
assertSnapshot( | ||
for: [.vivoNew, .movistar], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "This is a very long test text to check multiline", style: .title3) | ||
) | ||
} | ||
|
||
func testTitle3MultilineAndLink() { | ||
assertSnapshot( | ||
for: [.vivoNew, .movistar], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "This is a very long test text to check multiline", linkTitle: "Link text", style: .title3) | ||
) | ||
} | ||
|
||
func testTitle4() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. title 4 uses the same text preset for any brand so recording this for one brand is enough IMO |
||
assertSnapshot( | ||
for: [.vivoNew], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "Title text", style: .title4) | ||
) | ||
} | ||
|
||
func testTitle4WithLink() { | ||
assertSnapshot( | ||
for: [.vivoNew], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "Title text", linkTitle: "Link text", style: .title4) | ||
) | ||
} | ||
|
||
func testTitle4Multiline() { | ||
assertSnapshot( | ||
for: [.vivoNew], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "This is a very long test text to check multiline", style: .title4) | ||
) | ||
} | ||
|
||
func testTitle4MultilineAndLink() { | ||
assertSnapshot( | ||
for: [.vivoNew], | ||
and: [.light], | ||
as: .image(on: .iPhoneSe), | ||
viewBuilder: makeSectionTitle(title: "This is a very long test text to check multiline", linkTitle: "Link text", style: .title4) | ||
) | ||
} | ||
} | ||
|
||
private extension TitleHeaderFooterViewTests { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,9 +55,51 @@ func assertSnapshotForAllBrandsAndStyles<View: UserInterfaceStyling, Format>( | |
} | ||
} | ||
|
||
func assertSnapshot<View: UserInterfaceStyling, Format>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. useful to select in which brand you want to record screenshots. It will save us a lot of screenshots, sizes, time,... (talked in our previous platform evolution meeting) |
||
for brands: [BrandStyle] = BrandStyle.allCases, | ||
and styles: [UIUserInterfaceStyle] = [.light, .dark], | ||
as snapshotting: Snapshotting<View, Format>, | ||
file: StaticString = #file, | ||
testName: String = #function, | ||
line: UInt = #line, | ||
viewBuilder: @autoclosure () -> View | ||
) { | ||
for brand in brands { | ||
MisticaConfig.brandStyle = brand | ||
|
||
for style in styles { | ||
var view = viewBuilder() | ||
view.overrideUserInterfaceStyle = style | ||
assertSnapshot( | ||
matching: view, | ||
as: snapshotting, | ||
named: "with-\(brand)-\(style.testSuffix)-style", | ||
file: file, | ||
testName: testName, | ||
line: line | ||
) | ||
} | ||
} | ||
} | ||
|
||
protocol UserInterfaceStyling { | ||
var overrideUserInterfaceStyle: UIUserInterfaceStyle { get set } | ||
} | ||
|
||
extension UIView: UserInterfaceStyling {} | ||
extension UIViewController: UserInterfaceStyling {} | ||
|
||
private extension UIUserInterfaceStyle { | ||
var testSuffix: String { | ||
switch self { | ||
case .unspecified: | ||
"unspecified" | ||
case .light: | ||
"light" | ||
case .dark: | ||
"dark" | ||
@unknown default: | ||
"unknown" | ||
} | ||
} | ||
} |
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.
new ones!