From 878c27f9b03939e74b1094256379e2a821efeb82 Mon Sep 17 00:00:00 2001 From: ludwiczakpawel Date: Tue, 14 Dec 2021 14:43:41 +0100 Subject: [PATCH] Crayons: buttons followup (#15720) * init * whoops * components update * components update * classes * variables * focus * test * spec * test * adjust colors and variables * buttons colors * Update app/javascript/crayons/CTAs/CTA.jsx Co-authored-by: Suzanne Aitchison * proptype * remove duplicated tests now in Cypress, enhance Cypress nav menu tests * premove unneeded viewport in test * remove focus check for now * classnames * Update app/javascript/crayons/CTAs/__stories__/CTAs.mdx Co-authored-by: Nick Taylor * Update app/javascript/crayons/CTAs/__stories__/CTAs.mdx Co-authored-by: Nick Taylor * better contrast * classname * add secondary button * variants + docs * docs updates * radius-full * variants oneof * the? * default values + extra stories * Apply suggestions from code review Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> Co-authored-by: Suzanne Aitchison Co-authored-by: Nick Taylor Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> --- .../stylesheets/components/buttons.scss | 7 +++ app/assets/stylesheets/components/links.scss | 10 ++++- app/assets/stylesheets/config/_colors.scss | 20 ++++++--- app/javascript/crayons/Buttons/Button.jsx | 8 ++-- .../crayons/Buttons/__stories__/Buttons.mdx | 43 ++++++++++++------ .../Buttons/__stories__/Buttons.stories.jsx | 45 ++++++++++++++++--- .../Buttons/__tests__/Buttons.test.jsx | 37 ++++++++++++++- .../__snapshots__/Buttons.test.jsx.snap | 2 + app/javascript/crayons/CTAs/CTA.jsx | 6 +-- .../crayons/CTAs/__stories__/CTAs.mdx | 5 +-- .../crayons/CTAs/__stories__/CTAs.stories.jsx | 10 ++++- app/javascript/crayons/Links/Link.jsx | 6 +-- .../crayons/Links/__stories__/Links.mdx | 7 ++- .../Links/__stories__/Links.stories.jsx | 25 +++++++++-- 14 files changed, 177 insertions(+), 54 deletions(-) diff --git a/app/assets/stylesheets/components/buttons.scss b/app/assets/stylesheets/components/buttons.scss index 1898074e8..73f53803c 100644 --- a/app/assets/stylesheets/components/buttons.scss +++ b/app/assets/stylesheets/components/buttons.scss @@ -513,6 +513,13 @@ label[class*='crayons-btn']:focus-within // label for input[type="file"] made to } } +.c-btn--secondary { + --bg: var(--btn-secondary-bg); + --bg-hover: var(--btn-secondary-bg-hover); + --color: var(--btn-secondary-color); + --color-hover: var(--btn-secondary-color-hover); +} + .c-btn--icon-alone { padding: var(--su-2); } diff --git a/app/assets/stylesheets/components/links.scss b/app/assets/stylesheets/components/links.scss index 2c36111dc..0ae04af92 100644 --- a/app/assets/stylesheets/components/links.scss +++ b/app/assets/stylesheets/components/links.scss @@ -92,7 +92,13 @@ .c-link--icon-left { .c-link__icon { - margin-right: var(--su-2); - margin-left: calc(var(--su-1) * -1); + margin-right: var(--su-1); + } + + &.c-link--block { + .c-link__icon { + margin-right: var(--su-2); + margin-left: calc(var(--su-1) * -1); + } } } diff --git a/app/assets/stylesheets/config/_colors.scss b/app/assets/stylesheets/config/_colors.scss index 068b27276..4d11e85b8 100644 --- a/app/assets/stylesheets/config/_colors.scss +++ b/app/assets/stylesheets/config/_colors.scss @@ -257,19 +257,25 @@ --btn-current-bg: var(--base-20); --btn-current-color: var(--base-100); - // Buttons: Default Destructive - --btn-destructive-bg: transparent; - --btn-destructive-bg-hover: var(--accent-danger-a10); - --btn-destructive-color: var(--accent-danger); - --btn-destructive-color-hover: var(--accent-danger-darker); - // Buttons: Primary --btn-primary-bg: var(--accent-brand); --btn-primary-bg-hover: var(--accent-brand-darker); --btn-primary-color: var(--base-inverted); --btn-primary-color-hover: var(--base-inverted); - // Buttons: Primary Destructive + // Buttons: Secondary + --btn-secondary-bg: var(--accent-brand-a10); + --btn-secondary-bg-hover: var(--accent-brand); + --btn-secondary-color: var(--accent-brand); + --btn-secondary-color-hover: var(--base-inverted); + + // Buttons: Destructive Default + --btn-destructive-bg: transparent; + --btn-destructive-bg-hover: var(--accent-danger-a10); + --btn-destructive-color: var(--accent-danger); + --btn-destructive-color-hover: var(--accent-danger-darker); + + // Buttons: Destructive Primary --btn-primary-destructive-bg: var(--accent-danger); --btn-primary-destructive-bg-hover: var(--accent-danger-darker); --btn-primary-destructive-color: var(--base-inverted); diff --git a/app/javascript/crayons/Buttons/Button.jsx b/app/javascript/crayons/Buttons/Button.jsx index 111f94482..0e7aa40d9 100644 --- a/app/javascript/crayons/Buttons/Button.jsx +++ b/app/javascript/crayons/Buttons/Button.jsx @@ -8,7 +8,7 @@ import { Icon } from '@crayons'; export const ButtonNew = (props) => { const { children, - primary, + variant = 'default', icon, rounded, destructive, @@ -30,8 +30,8 @@ export const ButtonNew = (props) => { }; const classes = classNames('c-btn', { - 'c-btn--primary': primary, - 'c-btn--destructive': destructive, + [`c-btn--${variant}`]: variant && variant !== 'default', + 'c-btn--destructive': destructive && variant !== 'secondary', 'c-btn--icon-left': icon && children, 'c-btn--icon-alone': icon && !children, 'crayons-tooltip__activator': tooltip, @@ -73,7 +73,7 @@ ButtonNew.displayName = 'ButtonNew'; ButtonNew.propTypes = { children: defaultChildrenPropTypes, - primary: PropTypes.bool, + variant: PropTypes.oneOf(['default', 'primary', 'secondary']), rounded: PropTypes.bool, destructive: PropTypes.bool, type: PropTypes.oneOf(['button', 'submit']), diff --git a/app/javascript/crayons/Buttons/__stories__/Buttons.mdx b/app/javascript/crayons/Buttons/__stories__/Buttons.mdx index ca2fb29a2..54046696b 100644 --- a/app/javascript/crayons/Buttons/__stories__/Buttons.mdx +++ b/app/javascript/crayons/Buttons/__stories__/Buttons.mdx @@ -14,35 +14,50 @@ That is: `` or `` should be used where navigation is elicited, an For example, a 'Submit' control on 'Sign in' view (form) would suit a `, + ); + const results = await axe(container); + + expect(results).toHaveNoViolations(); + }); + + it('has no accessibility errors in secondary variant', async () => { + const { container } = render( + , + ); + const results = await axe(container); + + expect(results).toHaveNoViolations(); + }); + it('has no accessibility errors when props provided', async () => { const { container } = render( - , ); @@ -30,7 +54,16 @@ describe('); + const { container } = render( + , + ); + expect(container.innerHTML).toMatchSnapshot(); + }); + + it('renders a secondary button', () => { + const { container } = render( + , + ); expect(container.innerHTML).toMatchSnapshot(); }); diff --git a/app/javascript/crayons/Buttons/__tests__/__snapshots__/Buttons.test.jsx.snap b/app/javascript/crayons/Buttons/__tests__/__snapshots__/Buttons.test.jsx.snap index 9ba8af636..d0ecddc6d 100644 --- a/app/javascript/crayons/Buttons/__tests__/__snapshots__/Buttons.test.jsx.snap +++ b/app/javascript/crayons/Buttons/__tests__/__snapshots__/Buttons.test.jsx.snap @@ -8,6 +8,8 @@ exports[`"`; +exports[`"`; + exports[`"`; exports[`"`; diff --git a/app/javascript/crayons/CTAs/CTA.jsx b/app/javascript/crayons/CTAs/CTA.jsx index a47089816..70aab87d0 100644 --- a/app/javascript/crayons/CTAs/CTA.jsx +++ b/app/javascript/crayons/CTAs/CTA.jsx @@ -8,14 +8,14 @@ export const CTA = (props) => { const { children, href = '#', - variant, + variant = 'default', icon, className, ...otherProps } = props; const classes = classNames('c-cta', { - [`c-cta--${variant}`]: variant, + [`c-cta--${variant}`]: variant && variant !== 'default', 'c-cta--icon-left': icon && children, [className]: className, }); @@ -38,7 +38,7 @@ export const CTA = (props) => { CTA.displayName = 'CTA'; CTA.propTypes = { - variant: PropTypes.oneOf([undefined, 'branded']), + variant: PropTypes.oneOf(['default', 'branded']), rounded: PropTypes.bool, href: PropTypes.string.isRequired, className: PropTypes.string, diff --git a/app/javascript/crayons/CTAs/__stories__/CTAs.mdx b/app/javascript/crayons/CTAs/__stories__/CTAs.mdx index 6b7d9f15f..06db101e6 100644 --- a/app/javascript/crayons/CTAs/__stories__/CTAs.mdx +++ b/app/javascript/crayons/CTAs/__stories__/CTAs.mdx @@ -16,12 +16,11 @@ Do not use `