Skip to content

Commit

Permalink
ForwardRef on Remaining Base Components (#75)
Browse files Browse the repository at this point in the history
* Forward ref on remaining base components

* adjust changelog

Co-authored-by: Guillaume Lambert <[email protected]>
  • Loading branch information
xdrdak and glambert authored Jun 12, 2020
1 parent f426595 commit 00153d0
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 60 deletions.
4 changes: 4 additions & 0 deletions packages/flame/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ Refer to the [CONTRIBUTING guide](https://github.com/lightspeed/flame/blob/maste

## [Unreleased]

### Breaking

- Use React.forwardRef on `<Button>` and `<Switch>` components ([#75](https://github.com/lightspeed/flame/pull/75))

### Deprecation Warning

- Alert icon prop will be removed in the next major feature release. Icons will be automatically assigned to an alert based on the variant used ([#82](https://github.com/lightspeed/flame/pull/82))
Expand Down
6 changes: 6 additions & 0 deletions packages/flame/src/Button/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,10 @@ describe('Button Component', () => {
getByTitle('Loading...');
expect(domNode).not.toBeVisible();
});

it('should forward the ref properly', () => {
const ref = React.createRef<HTMLButtonElement>();
customRender(<Button ref={ref}>Button with a ref attached to it</Button>);
expect(ref.current.type).toBe('button');
});
});
100 changes: 53 additions & 47 deletions packages/flame/src/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,53 +177,59 @@ export type ButtonProps = Merge<
/**
* Buttons are used to take action or confirm a decision. They help merchants get things done.
*/
export const Button: React.FunctionComponent<ButtonProps> = ({
loading,
children,
size,
noSpacing,
fill,
variant,
forcedState,
className,
disabled,
...restProps
}) => {
const iconAdjustments = loneIconAdjustments(children, size);

const nextChildren = React.Children.map(children, child => remapChild(child, size));
const LinkifiedButton = restProps.href
? ExtendedBaseButton.withComponent('a')
: ExtendedBaseButton;

const nextForceState = forcedState && `cr-button--${forcedState}`;
const isDisabled = loading || disabled;

// TODO: Need to rework or split off the link buttons from the main component.
// It causes some gnarly typing errors and overloads the component with even
// more useless logic, since emotion supports the `as` prop anyways...
return (
// @ts-ignore
<LinkifiedButton
isFillButton={fill}
size={size}
disabled={isDisabled}
variant={variant === 'default' ? 'neutral' : variant}
className={[className, nextForceState].join(' ')}
{...iconAdjustments}
{...(restProps as any)}
>
{loading && (
<SpinnerWrapper title="Loading...">
<StyledSpinner fill={fill} variant={variant} size={size} />
</SpinnerWrapper>
)}
<ChildWrapper noSpacing={noSpacing} isLoading={loading}>
{nextChildren}
</ChildWrapper>
</LinkifiedButton>
);
};
export const Button = React.forwardRef<HTMLButtonElement | HTMLAnchorElement, ButtonProps>(
(
{
loading,
children,
size,
noSpacing,
fill,
variant,
forcedState,
className,
disabled,
...restProps
},
ref,
) => {
const iconAdjustments = loneIconAdjustments(children, size);

const nextChildren = React.Children.map(children, child => remapChild(child, size));
const LinkifiedButton = restProps.href
? ExtendedBaseButton.withComponent('a')
: ExtendedBaseButton;

const nextForceState = forcedState && `cr-button--${forcedState}`;
const isDisabled = loading || disabled;

// TODO: Need to rework or split off the link buttons from the main component.
// It causes some gnarly typing errors and overloads the component with even
// more useless logic, since emotion supports the `as` prop anyways...
return (
// @ts-ignore
<LinkifiedButton
ref={ref}
isFillButton={fill}
size={size}
disabled={isDisabled}
variant={variant === 'default' ? 'neutral' : variant}
className={[className, nextForceState].join(' ')}
{...iconAdjustments}
{...(restProps as any)}
>
{loading && (
<SpinnerWrapper title="Loading...">
<StyledSpinner fill={fill} variant={variant} size={size} />
</SpinnerWrapper>
)}
<ChildWrapper noSpacing={noSpacing} isLoading={loading}>
{nextChildren}
</ChildWrapper>
</LinkifiedButton>
);
},
);

Button.defaultProps = {
size: 'medium',
Expand Down
6 changes: 6 additions & 0 deletions packages/flame/src/Switch/Switch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,10 @@ describe('<Switch />', () => {
expect(container.querySelectorAll('.custom-class')).toBeTruthy();
});
});

it('should forward the ref properly', () => {
const ref = React.createRef<HTMLInputElement>();
customRender(<Switch ref={ref} />);
expect(ref.current.type).toBe('checkbox');
});
});
34 changes: 21 additions & 13 deletions packages/flame/src/Switch/Switch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,25 @@ export type SwitchProps = Merge<
/**
* A toggleable control which stays on (or off) until manually triggered once more.
*/
export const Switch: React.FC<SwitchProps> = ({ className, checked, ...restProps }) => (
<WrapperLabel role="presentation" className={className}>
<SwitchInput type="checkbox" checked={checked} value={checked ? 1 : 0} {...restProps} />
<SwitchWrapper>
<SwitchSlider>
<SwitchSliderIcon />
</SwitchSlider>
<IconWrapper>
<Checkmark size="0.75rem" />
<Cross size="0.75rem" />
</IconWrapper>
</SwitchWrapper>
</WrapperLabel>
export const Switch = React.forwardRef<HTMLInputElement, SwitchProps>(
({ className, checked, ...restProps }, ref) => (
<WrapperLabel role="presentation" className={className}>
<SwitchInput
type="checkbox"
checked={checked}
value={checked ? 1 : 0}
ref={ref}
{...restProps}
/>
<SwitchWrapper>
<SwitchSlider>
<SwitchSliderIcon />
</SwitchSlider>
<IconWrapper>
<Checkmark size="0.75rem" />
<Cross size="0.75rem" />
</IconWrapper>
</SwitchWrapper>
</WrapperLabel>
),
);

0 comments on commit 00153d0

Please sign in to comment.