Created Crayons Button Preact component #6987

nickytonline · 2020-03-31T22:13:44Z

What type of PR is this? (check all applicable)

Description

Design system buttons are now available as Preact button components. See the latest Storybook to see button components in action.

By default the <Button /> component renders as an HTML button element, <button ... />, but you can also render it as an HTML anchor element, <a ... />, via the as prop of the <Button /> component. I used as as this is what styled-components used. I basically couldn't think of a better prop name. 😄

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Primary

<Button {...props} />

Secondary

<Button variant="secondary" {...props} />

Outlined

<Button variant="outlined" {...props} />

Danger

<Button variant="danger" {...props} />

Added tests?

yes
no, because they aren't needed
no, because I need help

Added to documentation?

docs.dev.to
readme
no documentation needed

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

ludwiczakpawel · 2020-04-01T10:56:00Z

whoops i didn't mean to approve, since it's a still draft ;p but whatever..

nickytonline · 2020-04-01T17:26:44Z

app/javascript/crayons/Button/__stories__/button.html.stories.jsx

@@ -46,16 +46,6 @@ Outlined.story = {
  name: 'outlined',
 };

-export const Text = () => (


This story for the HTML components really isn't necessary. Most buttons just have text in them.

nickytonline · 2020-04-01T17:27:33Z

app/javascript/crayons/Dropdown/__stories__/Dropdown.stories.jsx

@@ -5,7 +5,7 @@ import { Dropdown } from '@crayons';
 import './dropdown-css-helper.scss';

 export default {
-  title: 'Components/Dropdowns/JSX',
+  title: 'Components/Dropdowns',


For Preact components, no point on adding another folder in the hierarchy for the Storybook sidebar. It makes sense for HTML components, but not JSX.

nickytonline · 2020-04-01T17:28:04Z

app/javascript/crayons/index.js

@@ -1 +1,2 @@
+export * from '@crayons/Button';


Export all buttons so that they are accessible via.

import { Button } from '@crayons';

nickytonline · 2020-04-01T17:29:56Z

app/javascript/crayons/Button/Button.jsx

+  }
+
+  if (disabled) {
+    additionalClassNames += ' crayons-btn--disabled';


Only buttons that generated <a /> tags get this CSS class.

nickytonline · 2020-04-01T17:30:28Z

app/javascript/crayons/Button/Button.jsx

+    as === 'button' ? { type: buttonType, disabled } : { href: url };
+
+  return (
+    <ComponentName


ComponentName can be either button or a.

nickytonline · 2020-04-01T17:31:57Z

app/javascript/crayons/Button/Button.jsx

+  return additionalClassNames;
+}
+
+export const Button = ({


Default settings will give us a primary button.

nickytonline · 2020-04-01T17:32:40Z

app/javascript/crayons/Button/DangerButton.jsx

+import { defaultChildrenPropTypes } from '../../src/components/common-prop-types';
+import { Button } from './Button';
+
+export const DangerButton = ({


nickytonline · 2020-04-01T17:33:12Z

app/javascript/crayons/Button/OutlinedButton.jsx

+import { defaultChildrenPropTypes } from '../../src/components/common-prop-types';
+import { Button } from './Button';
+
+export const OutlinedButton = ({


nickytonline · 2020-04-01T17:33:41Z

app/javascript/crayons/Button/SecondaryButton.jsx

+import { defaultChildrenPropTypes } from '../../src/components/common-prop-types';
+import { Button } from './Button';
+
+export const SecondaryButton = ({


nickytonline · 2020-04-01T18:50:06Z

@ludwiczakpawel, just a couple of outstanding things to do for buttons. I still have to write some tests and I also noticed that hover and active styles work when a button is disabled. If I get time today, I will look at this, but if not and you have time when you start your day tomorrow, it shouldn't take too long.

ludwiczakpawel · 2020-04-02T08:58:54Z

Lemme know if still needs help with hover/disabled.

nickytonline · 2020-04-02T12:44:36Z

Lemme know if still needs help with hover/disabled.

Looking at it now. I'll ping you if I get stuck.

…button is rendered.

… properly.

nickytonline · 2020-04-02T19:40:48Z

The code climate issues are complaining about prop types being repetitive code. This is a false negative as it's very common for prop types to become repetitive.

…crayons-button-preact-component

nickytonline · 2020-04-02T20:21:50Z

@ludwiczakpawel, I did not add a variant for a design crayons button that is just text. It doesn't really make sense to me or is this just supposed to be a regular hyperlink?

<a href="#" class="... crayons-btn--text">Text Button label</a>

ludwiczakpawel

I checked the disabled states, looking good!

ludwiczakpawel · 2020-04-03T10:34:46Z

btw. while on it... maybe you could kill that hover effect we have: https://github.com/thepracticaldev/dev.to/blob/master/app/assets/stylesheets/scaffolds.scss#L68-L74 ? it's driving me crazy :D

…crayons-button-preact-component

nickytonline · 2020-04-03T13:01:47Z

btw. while on it... maybe you could kill that hover effect we have: /app/assets/stylesheets/scaffolds.scss@master#L68-L74 ? it's driving me crazy :D

done.

rhymes

Nice! Left some comments about naming and the presence of almost identical components (I know I voted for the separate components in Slack, but I didn't realize they were going to be identical :D).

rhymes · 2020-04-03T13:00:24Z

app/assets/stylesheets/components/buttons.scss

+  &:enabled:not(&--disabled):hover,
+  &:enabled:not(&--disabled):active,
+  &--active:enabled:not(&--disabled) {


boolean logic FTW :D

rhymes · 2020-04-03T13:01:05Z

app/javascript/crayons/Button/Button.jsx

+  children,
+  variant = 'primary',
+  as = 'button',
+  className,
+  icon,
+  url,
+  buttonType,
+  disabled,
+  onClick,
+  onMouseOver,
+  onMouseOut,
+  onFocus,
+  onBlur,


quite a lot of params :D

These are all properties of the props parameter, so technically it's one param. 😄 They are all potentially required. It's not that super uncommon. Think of an HTML element and all the potential attributes they can have. 😉

rhymes · 2020-04-03T13:02:47Z

app/javascript/crayons/Button/Button.jsx

+export const Button = ({
+  children,
+  variant = 'primary',
+  as = 'button',


mmm, I saw you note about using as but as I see it's immediately assigned to ComponentName as a const, what if we call it htmlElement instead of as or componentName ?

The funny thing about JSX is a component name has to be Pascal cased. So it does look odd setting const ComponentName = as;.

rhymes · 2020-04-03T13:04:22Z

app/javascript/crayons/Button/Button.jsx

+        variant,
+        className,
+        icon,
+        disabled: as === 'a' && disabled,


htmlElement === a looks cool

Instead of htmlElement, it actually makes sense to call it tagName as that's what it really is, https://developer.mozilla.org/en-US/docs/Web/API/Element/tagName.

Great! tagName which follows HTML conventions!

rhymes · 2020-04-03T13:06:58Z

app/javascript/crayons/Button/DangerButton.jsx

@@ -0,0 +1,32 @@
+import { h } from 'preact';


Ok, I'm sure you thought about it, but do we need these specialized files? They seem identical except for the name of the component. They all use <Button variant="VARIANT_NAME">, what if we just use Button ? And if and when we need one with lots of custom behavior, that can be added.

What do you think?

I decided to go with the separate buttons mainly because I thought from a DX perspective, <DangerButton /> would be more grokkable than <Button variant="danger" />, but this is not a deal breaker for me.

+1 on having less code in general :-) Also ripgrep is quite fast so rg Button | rg danger is not a big issue :D

Yeah. I asked some others on our Slack, and I think I will just go with <Button variant="..." /> without the additional layer.

Now there is only the <Button /> component and you simply use the variant prop to decide what type of button you want. e.g. <Button variant="danger">DANGER!</Button>

joshpuetz

So awesome: this will make building reusable button types in the feed so much easier!

pr-triage bot added the PR: draft bot applied label for PR's that are a work in progress label Mar 31, 2020

nickytonline requested a review from ludwiczakpawel March 31, 2020 22:13

rhymes changed the title ~~Nickytonline/added crayons button preact component~~ Added Crayons button Preact component Apr 1, 2020

ludwiczakpawel approved these changes Apr 1, 2020

View reviewed changes

nickytonline requested a review from ludwiczakpawel April 1, 2020 17:18

nickytonline force-pushed the nickytonline/added-crayons-button-preact-component branch from 6807825 to 26fe594 Compare April 1, 2020 17:25

nickytonline commented Apr 1, 2020

View reviewed changes

nickytonline force-pushed the nickytonline/added-crayons-button-preact-component branch from dc62782 to e3b44e5 Compare April 1, 2020 22:01

nickytonline requested review from rhymes and Ridhwana April 2, 2020 12:43

nickytonline marked this pull request as ready for review April 2, 2020 12:44

pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: draft bot applied label for PR's that are a work in progress labels Apr 2, 2020

nickytonline requested a review from Zhao-Andy April 2, 2020 12:45

nickytonline added 7 commits April 2, 2020 08:46

wip <Button > component.

ef34b85

wip button components.

c94a575

Fixed issue when icon does not exist.

833df34

Removed subfolder JSX for Preact components in Storybook sidebar.

d97c429

Touched up some Storybook stories.

07ca235

Now <Button /> component supports a or button for HTML.

f4066f8

Now the <Button /> component supports the disabled prop

c298f5f

nickytonline changed the title ~~[WIP] Added Crayons button Preact component~~ Added Crayons button Preact component Apr 2, 2020

pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Apr 2, 2020

Now active and hover states are disabled for <Button /> when an HTML …

e089eea

…button is rendered.

nickytonline changed the title ~~Added Crayons button Preact component~~ Created Crayons Button Preact component Apr 2, 2020

Now the disabled <Button /> component deactivates hover/active states…

c4f34dd

… properly.

Merge remote-tracking branch 'origin/master' into nickytonline/added-…

a78d6ca

…crayons-button-preact-component

nickytonline requested review from joshpuetz and removed request for Zhao-Andy April 3, 2020 10:09

ludwiczakpawel approved these changes Apr 3, 2020

View reviewed changes

pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Apr 3, 2020

Removed global button hover styling as per @ludwiczakpawel's request.

a9ea9d1

pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Apr 3, 2020

Merge remote-tracking branch 'origin/master' into nickytonline/added-…

9e0a591

…crayons-button-preact-component

rhymes reviewed Apr 3, 2020

View reviewed changes

nickytonline added 2 commits April 3, 2020 09:27

Renamed the as prop to tagName.

aa95a02

Removed components that were wrapping different variants.

ce7bbe8

joshpuetz approved these changes Apr 3, 2020

View reviewed changes

pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Apr 3, 2020

nickytonline merged commit d6e5808 into master Apr 3, 2020

nickytonline deleted the nickytonline/added-crayons-button-preact-component branch April 3, 2020 14:49

pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Apr 3, 2020

benhalpern mentioned this pull request Apr 3, 2020

Add text-decoration: none back in for anchor #7072

Merged

5 tasks

Created Crayons Button Preact component #6987

Created Crayons Button Preact component #6987

Uh oh!

Conversation

nickytonline commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this? (check all applicable)

Description

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Added tests?

Added to documentation?

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

Uh oh!

ludwiczakpawel commented Apr 1, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickytonline commented Apr 1, 2020

Uh oh!

ludwiczakpawel commented Apr 2, 2020

Uh oh!

nickytonline commented Apr 2, 2020

Uh oh!

nickytonline commented Apr 2, 2020

Uh oh!

nickytonline commented Apr 2, 2020

Uh oh!

ludwiczakpawel left a comment

Choose a reason for hiding this comment

Uh oh!

ludwiczakpawel commented Apr 3, 2020

Uh oh!

nickytonline commented Apr 3, 2020

Uh oh!

rhymes left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickytonline Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickytonline commented Mar 31, 2020 •

edited

Loading

rhymes left a comment •

edited

Loading

nickytonline Apr 3, 2020 •

edited

Loading