Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

feat(button): Replace feel="secondary" with color={colors.white} #65

Merged
merged 1 commit into from
Aug 12, 2019

Conversation

justinanastos
Copy link
Contributor

@justinanastos justinanastos commented Aug 12, 2019

@cheapsteak warned me about this weird API before; here we are putting it back.

This will allow us to take a single variant and easily generate a config for our buttons in engine.

Example:

import { Button as SpaceKitButton } from '@apollo/space-kit/Button';
import { colors } from '@apollo/space-kit/colors';

function getButtonColor(intention: Props['intention']) {
  switch (intention) {
    case 'danger':
      return colors.red.base;
    case 'cancel':
      return colors.white;
    default: {
      const _assertUnreachable: never = intention;
    }
  }
}

interface Props extends React.ComponentProps<typeof SpaceKitButton> {
  intention: 'danger' | 'cancel';
}

export const Button: React.FC<Props> = ({
  children,
  intention,
  ...otherProps
}) => (
  <SpaceKitButton color={getButtonColor(intention)} {...otherProps}>
    {children}
  </SpaceKitButton>
);

@justinanastos justinanastos added the minor Increment the minor version when merged label Aug 12, 2019
Copy link
Member

@cheapsteak cheapsteak left a comment

Choose a reason for hiding this comment

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

neat image

Never thought to intentionally use a never type before.
Any chance you have an article handy if I want to read more about it? Or was this self-discovered?

@justinanastos
Copy link
Contributor Author

@cheapsteak

neat image

Never thought to intentionally use a never type before.
Any chance you have an article handy if I want to read more about it? Or was this self-discovered?

I don't remember where I saw this, but this talks about it briefly. http://ideasintosoftware.com/exhaustive-switch-in-typescript/

We're using this in a few places in Engine right now. I love this pattern because it doesn't let you break things by accident.

If you can think of one, I'd much rather use this with a nested ternary versus a function with a switch, but I haven't thought about how.

@justinanastos justinanastos merged commit e96ebcb into master Aug 12, 2019
@justinanastos justinanastos deleted the justin/change-secondary-button-props branch August 12, 2019 16:23
@justinanastos
Copy link
Contributor Author

🚀 PR was released in v1.3.0 🚀

@justinanastos justinanastos added the released This issue/pull request has been released. label Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants