Skip to content
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

Add the DesignKit package to the project #6275

Merged
merged 4 commits into from
Jul 7, 2022
Merged

Conversation

pixlwave
Copy link
Member

@pixlwave pixlwave commented Jun 13, 2022

This PRs adopts the new DesignKit package from ElementX (#6276). It uses the package for both fonts and colours in the SwiftUI theme and ThemeV2 and removes the old DesignKit framework from the project.

Currently the colours come from element-design-tokens and the fonts live in the DesignKit package (along with some SwiftUI components which are unused in this initial PR).

DesignTokens uses a more modern architecture for our colours, adding the Dark Variant value by loading colours from an asset catalog. In ElementX we can use these colours directly and let the system handle all theming for us automatically. To make this work with Element iOS the following has been adopted:

  • SwiftUI: The VectorHostingController is updated to utilise the theme's UIUserInterfaceStyle. This means that the SwiftUI ElementColors can be used as is, with the theme dictating the correct colorScheme to render the view with.
  • UIKit: This PR implements an ElementUIColorsResolved class which takes the ElementUIColors from DesignTokens and resolves them for both light and dark modes. These resolved colours are then used in ThemeV2 replacing ColorsUIKit.

Technically the SwiftUI theme is now redundant as colours could be accessed via .element.accent and fonts via .element.body like we do in ElementX, but that would be a large update and needs further discussion as to whether we should actually do this in Element-iOS or not. Additionally I have (where possible) updated SwiftUI views to use the colours as named in compound but added additional properties to ElementUIColorsResolved to maintain source compatibility with the UIKit screens (as there are rather more of those that would need updating).

No screenshots as nothing has changed visually.

Comment on lines +47 to +50
public let quarterlyContent: UIColor
public let navigation: UIColor
public let tile: UIColor
public let separator: UIColor
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are colours that were in the theme, but not in compound.

@@ -17,6 +17,8 @@
import Foundation
import UIKit

// TODO: Move into element-design-tokens repo.

// Figma Avatar Sizes: https://www.figma.com/file/X4XTH9iS2KGJ2wFKDqkyed/Compound?node-id=1258%3A19678
public enum AvatarSize: Int {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can become tokens that are shared between all platforms.

@@ -51,7 +51,7 @@ struct BorderedInputFieldStyle: TextFieldStyle {
if (theme.identifier == ThemeIdentifier.dark) {
return (isEnabled ? theme.colors.primaryContent : theme.colors.tertiaryContent)
} else {
return (isEnabled ? theme.colors.primaryContent : theme.colors.quarterlyContent)
return (isEnabled ? theme.colors.primaryContent : theme.colors.quaternaryContent)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there were different names for the same colour. I have updated SwiftUI to be consistent with compound/DesignKit but left both available for UIKit.

@@ -76,7 +76,7 @@ struct MultilineTextField: View {
.overlay(rect.stroke(borderColor, lineWidth: borderWidth))
.introspectTextView { textView in
textView.textColor = UIColor(textColor)
textView.font = theme.fonts.uiFonts.callout
textView.font = .element.callout
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is no longer necessary to have a uiFonts property on the SwiftUI themes as these can be referenced statically now.

@@ -39,15 +39,15 @@ struct SearchBar: View {
}
.padding(8)
.padding(.horizontal, 25)
.background(theme.colors.navigation)
.background(theme.colors.system)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as quarterly/quaternary content.

@github-actions
Copy link

github-actions bot commented Jun 13, 2022

📱 Scan the QR code below to install the build for this PR.
🔒 This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.

QR code

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/zCXCXF

Fix the confetti colour when using DesignKit.
Pin swift packages.
Fix UI tests target.
@pixlwave pixlwave marked this pull request as ready for review July 1, 2022 14:10
@pixlwave pixlwave requested review from a team and Anderas and removed request for a team July 1, 2022 14:10
@sonarcloud
Copy link

sonarcloud bot commented Jul 1, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@Anderas Anderas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen much of the DesignKit project before so had to quickly catch up to speed. It's great to see this moved over and reuse a bunch of code

@pixlwave pixlwave merged commit 2398c15 into develop Jul 7, 2022
@pixlwave pixlwave deleted the doug/43_design_kit branch July 7, 2022 14:42
pixlwave added a commit that referenced this pull request Jul 8, 2022
pixlwave added a commit that referenced this pull request Jul 8, 2022
pixlwave added a commit that referenced this pull request Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants