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

[Theming] Add TYPOGRAPHY_CONSTANTS #520

Merged
merged 4 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/constants/themes/primaryTheme.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import PRIMARY_COLORS from '../colors/primary';
import PRIMARY_FONTS from '../fonts/primary';
import { PRIMARY_TYPOGRAPHY } from '../typography/primaryTypography';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general I think named exports are preferable


export const primaryTheme = {
__type: 'primary',
BOX_SHADOW: {},
COLORS: PRIMARY_COLORS,
FONTS: PRIMARY_FONTS,
TYPOGRAPHY: PRIMARY_TYPOGRAPHY,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Went with just TYPOGRAPHY instead of TYPOGRAPHY_CONSTANTS since we're accessing it off the theme now, so it is more obvious they are constants: there's no need to distinguish between TYPOGRAPHY and TYPOGRAPHY_CONSTANTS since they're not being imported from the same place like we currently do.

} as const;
2 changes: 2 additions & 0 deletions src/constants/themes/secondaryTheme.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import SECONDARY_COLORS from '../colors/secondary';
import SECONDARY_FONTS from '../fonts/secondary';
import { SECONDARY_TYPOGRAPHY } from '../typography/secondaryTypography';

export const secondaryTheme = {
__type: 'secondary',
BOX_SHADOW: {},
COLORS: SECONDARY_COLORS,
FONTS: SECONDARY_FONTS,
TYPOGRAPHY: SECONDARY_TYPOGRAPHY,
} as const;
10 changes: 10 additions & 0 deletions src/constants/themes/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import compareObjectsKeysLength from 'src/utils/compareObjectsKeysLength';

import { primaryTheme } from './primaryTheme';
import { secondaryTheme } from './secondaryTheme';

describe('themes', () => {
it('primary and secondary themes have the same number of properties', () => {
expect(compareObjectsKeysLength(primaryTheme, secondaryTheme)).toBe(true);
});
});
5 changes: 5 additions & 0 deletions src/constants/themes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ type Colors = typeof primaryTheme['COLORS'] | typeof secondaryTheme['COLORS'];

type Fonts = typeof primaryTheme['FONTS'] | typeof secondaryTheme['FONTS'];

type Typography =
| typeof primaryTheme['TYPOGRAPHY']
| typeof secondaryTheme['TYPOGRAPHY'];

export type ThemeColors = valueof<Colors>;

export const COLORS_PROP_TYPES = PropTypes.oneOf([
Expand All @@ -23,4 +27,5 @@ export type ThemeType = {
BOX_SHADOW: BoxShadow;
COLORS: Colors;
FONTS: Fonts;
TYPOGRAPHY: Typography;
};
18 changes: 18 additions & 0 deletions src/constants/typography/primaryTypography.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
const fontSize = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These values are duplicated from src/constants/typography/index because I don't think we should be wrapping them in throwOnUndefinedProperty. The value-add is less certain the more we have converted over to TS.

display: '2.25rem', // 36px
heading: '1.5rem', // 24px
title: '1.25rem', // 20px
body: '1rem', // 16px
caption: '0.875rem', // 14px
label: '0.75rem', // 12px
} as const;

const fontWeight = {
bold: 'bold',
normal: 400,
} as const;

export const PRIMARY_TYPOGRAPHY = {
fontSize,
fontWeight,
} as const;
19 changes: 19 additions & 0 deletions src/constants/typography/secondaryTypography.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// TODO: Update when values are finalized
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeping the values the same for now, rather than do empty strings like we are for Colors.

const fontSize = {
display: '2.25rem', // 36px
heading: '1.5rem', // 24px
title: '1.25rem', // 20px
body: '1rem', // 16px
caption: '0.875rem', // 14px
label: '0.75rem', // 12px
} as const;

const fontWeight = {
bold: 'bold',
normal: 400,
} as const;

export const SECONDARY_TYPOGRAPHY = {
fontSize,
fontWeight,
} as const;
12 changes: 12 additions & 0 deletions src/constants/typography/test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import compareObjectsKeysLength from 'src/utils/compareObjectsKeysLength';

import { PRIMARY_TYPOGRAPHY } from './primaryTypography';
import { SECONDARY_TYPOGRAPHY } from './secondaryTypography';

describe('theme typography', () => {
it('primary and secondary typography have the same number of properties', () => {
expect(
compareObjectsKeysLength(PRIMARY_TYPOGRAPHY, SECONDARY_TYPOGRAPHY),
).toBe(true);
});
});
42 changes: 39 additions & 3 deletions src/utils/compareObjectsKeysLength/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,42 @@
// eslint-disable-next-line @typescript-eslint/consistent-indexed-object-style
interface RecursiveObjectType {
[key: string]: string | number | this;
}

const stringifyObjects = (
obj1: RecursiveObjectType,
obj2: RecursiveObjectType,
) => `\n\nobj1: ${JSON.stringify(obj1)}\n\nobj2:${JSON.stringify(obj2)}`;

const compareObjectsKeysLength = (
object1: Record<string, unknown>,
object2: Record<string, unknown>,
) => Object.keys(object1).length === Object.keys(object2).length;
obj1: RecursiveObjectType,
obj2: RecursiveObjectType,
): boolean => {
if (Object.keys(obj1).length !== Object.keys(obj2).length) {
console.error(`Object keys do not match:${stringifyObjects(obj1, obj2)}`);
return false;
}

return Object.keys(obj1).every((key) => {
const obj1Val = obj1[key];
const obj2Val = obj2[key];

// Test deep key equality recursively
if (typeof obj1Val === 'object' && typeof obj2Val === 'object') {
return compareObjectsKeysLength(obj1Val, obj2Val);
}

// Check if property value is null or undefined, neither of which are valid
// theme values, which means there was not a complementary key found
if (obj2Val == null) {
console.error(
`Key "${key}" missing in obj2:${stringifyObjects(obj1, obj2)}`,
);
return false;
}

return true;
});
};

export default compareObjectsKeysLength;