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

Updating colors and typography to use theme objects #4481

Merged
merged 2 commits into from
Jun 22, 2022

Conversation

georgewrmarshall
Copy link
Contributor

@georgewrmarshall georgewrmarshall commented Jun 8, 2022

Description

Updating colors and typography objects to use the lightTheme and darkTheme objects from design tokens v1.7. The next step would be to add a theme object to the context so the root theme can be changed and passed through the provider instead of individual objects.

Dependencies
MetaMask/design-tokens#148 has to be merged and package updated in mobile before this can be merged. ✅

Checklist

  • There is a related GitHub issue
  • [x] Tests are included if applicable NA
  • [x] Any added code is fully documented NA

Screenshots/Recordings

Issue
#4473

@georgewrmarshall georgewrmarshall added the DO-NOT-MERGE Pull requests that should not be merged label Jun 8, 2022
@georgewrmarshall georgewrmarshall self-assigned this Jun 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2022

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@georgewrmarshall georgewrmarshall changed the title Updating colors and typography to use theme objects Updating colors and typography to use theme objects (WIP) Jun 8, 2022
app/util/theme/index.ts Outdated Show resolved Hide resolved
@georgewrmarshall georgewrmarshall marked this pull request as ready for review June 21, 2022 23:52
@georgewrmarshall georgewrmarshall requested a review from a team as a code owner June 21, 2022 23:52
@georgewrmarshall georgewrmarshall added area-UI and removed DO-NOT-MERGE Pull requests that should not be merged labels Jun 21, 2022
@georgewrmarshall georgewrmarshall changed the title Updating colors and typography to use theme objects (WIP) Updating colors and typography to use theme objects Jun 22, 2022
@georgewrmarshall georgewrmarshall merged commit ab5b45e into main Jun 22, 2022
@georgewrmarshall georgewrmarshall deleted the update/4473/theme-hook branch June 22, 2022 23:16
@github-actions github-actions bot locked and limited conversation to collaborators Jun 22, 2022
import { ThemeTypography } from '@metamask/design-tokens/dist/js/typography/types';

// TODO: This should probably be defined from @metamask/design-token library
export type Colors = any;
Copy link
Member

Choose a reason for hiding this comment

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

Removing this type is breaking our ts files.
I think it should have been kept as Colors: DesignTokenTheme['colors'];.

We use them in the aggregator views:
Screen Shot 2022-06-28 at 10 38 42

Do we have type check enabled? This should have been caught at that stage.

@Cal-L Cal-L added the release-5.4.0 PRs for v5.4.0 release label Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-UI release-5.4.0 PRs for v5.4.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants