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.

Sign up for GitHub

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

Jump to bottom

[Theme] - Change Primary Buttons colors for Secondary Theme #651

Merged
merged 9 commits into from
Dec 17, 2020
Merged

[Theme] - Change Primary Buttons colors for Secondary Theme #651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7acf6f8
change secondary theme buttons colors
daigof Dec 16, 2020
07dadc7
disabled button color border
daigof Dec 16, 2020
e96b584
refactor out theme styles
daigof Dec 17, 2020
9d3e0dd
Merge branch 'master' into button-theme-recolor
daigof Dec 17, 2020
01a4e1d
Merge branch 'master' into refactor-out-types-conditional
daigof Dec 17, 2020
f3469d8
restore font weight position
daigof Dec 17, 2020
a79a181
Merge branch 'refactor-out-types-conditional' into button-theme-recolor
daigof Dec 17, 2020
2206e6b
added comment
daigof Dec 17, 2020
208d66a
update snapshot build
daigof Dec 17, 2020
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
62 changes: 38 additions & 24 deletions .size-snapshot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"bundle.js": {
"bundled": 838982,
"minified": 769892,
"gzipped": 89891
"bundled": 837958,
"minified": 768341,
"gzipped": 90340
},
"bundle.umd.js": {
"bundled": 848766,
"minified": 746208,
"gzipped": 85979
"bundled": 847782,
"minified": 744431,
"gzipped": 86322
},
"constants/zScale/index.js": {
"bundled": 234,
Expand Down Expand Up @@ -584,16 +584,16 @@
}
},
"shared-components/typography/index.js": {
"bundled": 59378,
"minified": 58060,
"gzipped": 3862,
"bundled": 55970,
"minified": 54854,
"gzipped": 3667,
"treeshaked": {
"rollup": {
"code": 3046,
"import_statements": 119
"code": 2977,
"import_statements": 220
},
"webpack": {
"code": 4232
"code": 4267
}
}
},
Expand Down Expand Up @@ -1326,16 +1326,16 @@
}
},
"shared-components/button/shared-components/loader/style.js": {
"bundled": 6670,
"minified": 5901,
"gzipped": 2568,
"bundled": 6936,
"minified": 6161,
"gzipped": 2632,
"treeshaked": {
"rollup": {
"code": 1710,
"import_statements": 184
"code": 1783,
"import_statements": 267
},
"webpack": {
"code": 2819
"code": 2935
}
}
},
Expand Down Expand Up @@ -1424,16 +1424,16 @@
}
},
"shared-components/button/style.js": {
"bundled": 35847,
"minified": 33965,
"gzipped": 6765,
"bundled": 37044,
"minified": 35080,
"gzipped": 7072,
"treeshaked": {
"rollup": {
"code": 4654,
"import_statements": 261
"code": 4751,
"import_statements": 365
},
"webpack": {
"code": 6047
"code": 6184
}
}
},
Expand Down Expand Up @@ -1828,5 +1828,19 @@
"code": 951
}
}
},
"utils/themeStyles/index.js": {
"bundled": 1374,
"minified": 710,
"gzipped": 273,
"treeshaked": {
"rollup": {
"code": 0,
"import_statements": 0
},
"webpack": {
"code": 951
}
}
}
}
3 changes: 2 additions & 1 deletion src/shared-components/button/shared-components/loader/style.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { keyframes } from '@emotion/core';

import { ButtonTypeWithAction } from '../..';
import { ThemeColors, ThemeType } from '../../../../constants';
import { primaryButtonLoadingBackgroundColor } from '../../../../utils/themeStyles';

const statefulLoader = keyframes`
0% { opacity: 1; transform: translate3d(0, 0, 0) scale(1, 1); }
Expand All @@ -13,7 +14,7 @@ const statefulLoader = keyframes`
`;

const primaryLoadingStyles = (theme: ThemeType) => `
background-color: ${theme.COLORS.white};
background-color: ${primaryButtonLoadingBackgroundColor(theme)};
`;

const accentLoadingStyles = (buttonColor: ThemeColors) => `
Expand Down
25 changes: 18 additions & 7 deletions src/shared-components/button/style.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,35 @@ import tinycolor from 'tinycolor2';
import { style as TYPOGRAPHY_STYLE } from '../typography';
import { ANIMATION, SPACER, ThemeColors, ThemeType } from '../../constants';
import { textColorsAssociatedWithColors } from './constants';
import {
primaryButtonFontColor,
primaryButtonBackgroundColor,
} from '../../utils/themeStyles';

import { ButtonTypeWithAction } from '.';

const primaryStyles = (buttonColor: ThemeColors, theme: ThemeType) => `
background-color: ${buttonColor};
border-color: ${buttonColor};
color: ${theme.COLORS.white};
fill: ${theme.COLORS.white};
const primaryStyles = (buttonColor: ThemeColors, theme: ThemeType) => {
const backgroundColor = primaryButtonBackgroundColor(theme, buttonColor);
const fontColor = primaryButtonFontColor(theme);

return `
background-color: ${backgroundColor};
border-color: ${backgroundColor};
color: ${fontColor};
fill: ${fontColor};

&:visited,
&:hover {
opacity: 0.8;
}

&:focus,
&:not([href]):not([tabindex]):hover,
&:not([href]):not([tabindex]):focus {
color: ${theme.COLORS.white};
color: ${fontColor};
}
`;
};

const secondaryStyles = (
isLoading: boolean,
Expand Down Expand Up @@ -113,7 +124,7 @@ const loadingStyles = `

const disabledStyles = (theme: ThemeType) => `
background-color: ${theme.COLORS.defaultLight};
border-color: ${theme.COLORS.border};
border-color: ${theme.COLORS.defaultLight};
daigof marked this conversation as resolved.
Show resolved Hide resolved
color: ${theme.COLORS.textDisabled};
cursor: not-allowed;
fill: ${theme.COLORS.textDisabled};
Expand Down
17 changes: 5 additions & 12 deletions src/shared-components/typography/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@ import round from 'lodash.round';

import { withDeprecationWarning } from '../../utils';
import { ThemeType } from '../../constants';

/**
* We use theme.FONTS.baseFont for all primary styles, but use a
* different secondary font for Display, Heading, and Title styles
*/
const setSecondaryHeadingFont = (theme: ThemeType) =>
theme.__type === 'secondary' ? `font-family: ${theme.FONTS.headerFont};` : '';
import {
setSecondaryHeadingFont,
setButtonStyleFontWeight,
} from '../../utils/themeStyles';

const displayStyle = (theme: ThemeType) => `
color: ${theme.COLORS.primary};
Expand Down Expand Up @@ -71,11 +68,7 @@ const buttonStyle = (theme: ThemeType) => `
color: ${theme.COLORS.primaryTint1};
font-size: ${theme.TYPOGRAPHY.fontSize.button};
line-height: ${round(20 / 12, 2)};
${
theme.__type === 'primary'
? `font-weight: ${theme.TYPOGRAPHY.fontWeight.bold};`
: ''
}
${setButtonStyleFontWeight(theme)}
letter-spacing: 1px;
text-transform: uppercase;
`;
Expand Down
34 changes: 34 additions & 0 deletions src/utils/themeStyles/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Any conditional style based on theme.__type should be on this file
*/
import { ThemeColors, ThemeType } from '../../constants';

export const primaryButtonFontColor = (theme: ThemeType) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great! I wonder if we should add a comment at the top of the file that explains that any ternary that is checking theme.__type should probably be moved into a util function in this file? (With the one exception being the Icon component, as you left it).

This isn't not a blocking comment--thank you for this cleanup 🙂

theme.__type === 'primary' ? theme.COLORS.white : theme.COLORS.primary;

export const primaryButtonBackgroundColor = (
theme: ThemeType,
buttonColor: ThemeColors,
) => {
if (buttonColor === theme.COLORS.primary && theme.__type === 'secondary') {
return theme.COLORS.secondary;
}

// If buttonColor is not COLORS.primary then it is custom, return as is
return buttonColor;
};

export const primaryButtonLoadingBackgroundColor = (theme: ThemeType) =>
theme.__type === 'primary' ? theme.COLORS.white : theme.COLORS.primary;

/**
* We use theme.FONTS.baseFont for all primary styles, but use a
* different secondary font for Display, Heading, and Title styles
*/
export const setSecondaryHeadingFont = (theme: ThemeType) =>
theme.__type === 'secondary' ? `font-family: ${theme.FONTS.headerFont};` : '';

export const setButtonStyleFontWeight = (theme: ThemeType) =>
theme.__type === 'primary'
? `font-weight: ${theme.TYPOGRAPHY.fontWeight.bold};`
: '';