OakButton component#592
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| iconSize: sizeConfig.iconSize, | ||
| }; | ||
|
|
||
| switch (variant) { |
There was a problem hiding this comment.
This approach allows different underlying components for different variant.
In oakButtonConfig the same values are accepted for each variant but in here we only pass the values relevant to the underlying component.
There is a decision to be made here, we could:
- unify the underlying components' API so they accept the same set of props, or
- update
oakButtonConfigtype to require only values relevant for eachvariant
Values which are currently needed in InternalShadowRectButton but not in InternalShadowRoundButton:
defaultBackgrounddefaultBorderColorhoverBackgroundhoverBorderColordisabledBackgrounddisabledBorderColorhoverShadowhoverUnderline
Values which are needed in InternalShadowRoundButton but not in InternalShadowRectButton:
defaultIconColordefaultIconBackgrounddefaultIconBorderColorhoverIconBackgroundhoverIconBorderColordisabledIconColordisabledIconBackgrounddisabledIconBorderColoriconBackgroundSize
There was a problem hiding this comment.
Dropping it here for a reference - this is how the oakButtonConfig would look like if we decided not to make any changes to the underlying components:
export type InternalShadowRectButtonColorProps = {
defaultTextColor: OakUiRoleToken;
defaultBackground: OakUiRoleToken;
defaultBorderColor: OakUiRoleToken;
hoverTextColor: OakUiRoleToken;
hoverBackground: OakUiRoleToken;
hoverBorderColor: OakUiRoleToken;
disabledTextColor: OakUiRoleToken;
disabledBackground: OakUiRoleToken;
disabledBorderColor: OakUiRoleToken;
hoverShadow: OakDropShadowToken | null;
};
export type InternalShadowRoundButtonColorProps = {
defaultTextColor: OakUiRoleToken;
defaultIconColor: OakUiRoleToken;
defaultIconBackground: OakUiRoleToken;
defaultIconBorderColor: OakUiRoleToken;
hoverTextColor: OakUiRoleToken;
hoverIconColor: OakUiRoleToken;
hoverIconBackground: OakUiRoleToken;
hoverIconBorderColor: OakUiRoleToken;
disabledTextColor:OakUiRoleToken;
disabledIconColor: OakUiRoleToken;
disabledIconBackground: OakUiRoleToken;
disabledIconBorderColor: OakUiRoleToken;
hoverShadow: OakDropShadowToken | null;
};
/**
* Specifies props for each variant taking into acccount an underlying component
*/
type OakButtonVariantConfig = {
primary: {
colorSchemes: {
primary: InternalShadowRectButtonColorProps;
inverted: InternalShadowRectButtonColorProps;
},
},
secondary: {
colorSchemes: {
primary: InternalShadowRectButtonColorProps;
},
},
tertiary: {
colorSchemes: {
primary: InternalShadowRoundButtonColorProps;
inverted: InternalShadowRoundButtonColorProps;
transparent: InternalShadowRoundButtonColorProps;
},
},
};
in OakButton we would then use:
switch (variant) {
case "primary":
case "secondary": {
const rectColorScheme = colorSchemeConfig as InternalShadowRectButtonColorProps;
return (
<InternalShadowRectButton
element={element ?? "button"}
defaultTextColor={rectColorScheme.defaultTextColor}
defaultBackground={rectColorScheme.defaultBackground}
defaultBorderColor={rectColorScheme.defaultBorderColor}
hoverTextColor={rectColorScheme.hoverTextColor}
hoverBackground={rectColorScheme.hoverBackground}
hoverBorderColor={rectColorScheme.hoverBorderColor}
disabledTextColor={rectColorScheme.disabledTextColor}
disabledBackground={rectColorScheme.disabledBackground}
disabledBorderColor={rectColorScheme.disabledBorderColor}
hoverShadow={rectColorScheme.hoverShadow}
hoverUnderline
{...sharedSizeProps}
{...rest}
>
{children}
</InternalShadowRectButton>
);
}
case "tertiary": {
const roundColorScheme = colorSchemeConfig as InternalShadowRoundButtonColorProps;
return (
<InternalShadowRoundButton
element={element ?? "button"}
defaultTextColor={roundColorScheme.defaultTextColor}
defaultIconColor={roundColorScheme.defaultIconColor}
defaultIconBackground={roundColorScheme.defaultIconBackground}
defaultIconBorderColor={roundColorScheme.defaultIconBorderColor}
hoverTextColor={roundColorScheme.hoverTextColor}
hoverIconColor={roundColorScheme.hoverIconColor}
hoverIconBackground={roundColorScheme.hoverIconBackground}
hoverIconBorderColor={roundColorScheme.hoverIconBorderColor}
disabledTextColor={roundColorScheme.disabledTextColor}
disabledIconColor={roundColorScheme.disabledIconColor}
disabledIconBackground={roundColorScheme.disabledIconBackground}
disabledIconBorderColor={roundColorScheme.disabledIconBorderColor}
iconBackgroundSize={sizeConfig.iconBackgroundSize}
hoverDropShadow={colorSchemeConfig.hoverShadow}
{...sharedSizeProps}
{...rest}
>
{children}
</InternalShadowRoundButton>
);
}
}
4e907f0 to
ac70d55
Compare
ac70d55 to
5e6b45f
Compare
|
Adding this comment for some general feedback and discussion points that I think we need to happen Thoughts
Proposed Next Steps
|
charsville
left a comment
There was a problem hiding this comment.
See comment on PR -- I think we should discuss and resolve this before this PR is ready for merge
|
Adding these links to the refactored button components in the Design Kit in Figma: |
Add your PR description below
OakButtoncomponent as a proof of concept for Oak Components Standards doc and introducedcolorScheme,sizeandvariantAPIOakPrimaryButtonOakPrimaryInvertedButtonOakSecondaryButtonOakSmallPrimaryButtonOakSmallPrimaryInvertedButtonOakSmallSecondaryButtonOakTertiaryButtonOakTertiaryInvertedButtonOakButtonAwaiting design decision
OakSmallTertiaryInvertedButtonOakButtonLink to the design doc
https://www.figma.com/design/YcWQMMhHPVVmc47cHHEEAl/Oak-Design-Kit?node-id=3459-11008&p=f&m=dev
A link to the component in the deployment preview
https://oak-components-storybook-git-oak-button-component.vercel.thenational.academy/?path=/docs/components-buttons-oakbutton--docs
Testing instructions
ACs