Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Button Group] Add outlined prop #6788

Merged
merged 5 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/core/changelog/@unreleased/pr-6788.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: '[Button Group] Add outlined prop'
links:
- https://github.com/palantir/blueprint/pull/6788
33 changes: 30 additions & 3 deletions packages/core/src/components/button/_button-group.scss
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Markup:
.#{$ns}-fill - Buttons expand equally to fill container
.#{$ns}-large - Use large buttons
.#{$ns}-minimal - Use minimal buttons
.#{$ns}-outlined - Use outlined buttons
.#{$ns}-vertical - Vertical layout

Styleguide button-group
Expand Down Expand Up @@ -96,7 +97,7 @@ Styleguide button-group
}

// support wrapping buttons in a tooltip, which adds a wrapper element
&:not(.#{$ns}-minimal) {
&:not(.#{$ns}-minimal), &.#{$ns}-outlined {
> .#{$ns}-popover-wrapper:not(:first-child) .#{$ns}-button,
> .#{$ns}-button:not(:first-child) {
border-bottom-left-radius: 0;
Expand All @@ -107,11 +108,17 @@ Styleguide button-group
> .#{$ns}-button:not(:last-child) {
border-bottom-right-radius: 0;
border-top-right-radius: 0;
}
}

&:not(.#{$ns}-minimal):not(.#{$ns}-outlined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed? I'm seeing the button group grow in size when outlined which I don't think is intentional

(added some extra buttons locally to test)

vertical (extra 1px per button, expect for last is what I'd assume):
Screenshot 2024-07-03 at 11 31 45 AM

Screenshot 2024-07-03 at 11 31 41 AM

horizontal (2px per button):
Screenshot 2024-07-03 at 11 33 55 AM

Screenshot 2024-07-03 at 11 34 00 AM

> .#{$ns}-popover-wrapper:not(:last-child) .#{$ns}-button,
> .#{$ns}-button:not(:last-child) {
margin-right: -$button-border-width;
}
}

&.#{$ns}-minimal {
&.#{$ns}-minimal, &.#{$ns}-outlined {
.#{$ns}-button {
@include pt-button-minimal();
}
Expand All @@ -134,6 +141,18 @@ Styleguide button-group
}
}

&.#{$ns}-outlined {
> .#{$ns}-button {
@include pt-button-outlined();
}

&:not(.#{$ns}-vertical) {
> .#{$ns}-button:not(:last-child) {
border-right: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

here I think none instead of transparent color actually helps, since the Button component itself currently grows 2px when outlined. this brings that down to 1px at least

}
}
}

.#{$ns}-popover-wrapper,
.#{$ns}-popover-target {
display: flex;
Expand Down Expand Up @@ -213,7 +232,7 @@ Styleguide button-group
width: 100%;
}

&:not(.#{$ns}-minimal) {
&:not(.#{$ns}-minimal), &.#{$ns}-outlined {
> .#{$ns}-popover-wrapper:first-child .#{$ns}-button,
> .#{$ns}-button:first-child {
border-radius: $pt-border-radius $pt-border-radius 0 0;
Expand All @@ -223,12 +242,20 @@ Styleguide button-group
> .#{$ns}-button:last-child {
border-radius: 0 0 $pt-border-radius $pt-border-radius;
}
}

&:not(.#{$ns}-minimal):not(.#{$ns}-outlined) {
> .#{$ns}-popover-wrapper:not(:last-child) .#{$ns}-button,
> .#{$ns}-button:not(:last-child) {
margin-bottom: -$button-border-width;
}
}

&.#{$ns}-outlined {
> .#{$ns}-button:not(:last-child) {
border-bottom: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

could we do a transparent color border instead of none here? border-bottom-color: transparent - it looks like that should fix any small shifts when toggling this prop

}
}
}

&.#{$ns}-align-left .#{$ns}-button {
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/components/button/buttonGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ export interface ButtonGroupProps extends Props, HTMLDivProps, React.RefAttribut
*/
minimal?: boolean;

/**
* Whether the child buttons should use outlined styles.
*
* @default false
*/
outlined?: boolean;

/**
* Whether the child buttons should appear with large styling.
*
Expand All @@ -70,13 +77,14 @@ export interface ButtonGroupProps extends Props, HTMLDivProps, React.RefAttribut
*/
export const ButtonGroup: React.FC<ButtonGroupProps> = React.forwardRef<HTMLDivElement, ButtonGroupProps>(
(props, ref) => {
const { alignText, className, fill, minimal, large, vertical, ...htmlProps } = props;
const { alignText, className, fill, minimal, outlined, large, vertical, ...htmlProps } = props;
const buttonGroupClasses = classNames(
Classes.BUTTON_GROUP,
{
[Classes.FILL]: fill,
[Classes.LARGE]: large,
[Classes.MINIMAL]: minimal,
[Classes.OUTLINED]: outlined,
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

[Classes.VERTICAL]: vertical,
},
Classes.alignmentClass(alignText),
Expand Down
5 changes: 5 additions & 0 deletions packages/docs-app/changelog/@unreleased/pr-6788.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: '[Button Group] Add outlined prop'
links:
- https://github.com/palantir/blueprint/pull/6788
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface ButtonGroupExampleState {
iconOnly: boolean;
intent: Intent;
minimal: boolean;
outlined: boolean;
large: boolean;
vertical: boolean;
}
Expand All @@ -51,6 +52,7 @@ export class ButtonGroupExample extends React.PureComponent<ExampleProps, Button
intent: Intent.NONE,
large: false,
minimal: false,
outlined: false,
vertical: false,
};

Expand All @@ -64,6 +66,8 @@ export class ButtonGroupExample extends React.PureComponent<ExampleProps, Button

private handleMinimalChange = handleBooleanChange(minimal => this.setState({ minimal }));

private handleOutlinedChange = handleBooleanChange(outlined => this.setState({ outlined }));

private handleVerticalChange = handleBooleanChange(vertical => this.setState({ vertical }));

public render() {
Expand Down Expand Up @@ -96,6 +100,7 @@ export class ButtonGroupExample extends React.PureComponent<ExampleProps, Button
<Switch checked={this.state.fill} label="Fill" onChange={this.handleFillChange} />
<Switch checked={this.state.large} label="Large" onChange={this.handleLargeChange} />
<Switch checked={this.state.minimal} label="Minimal" onChange={this.handleMinimalChange} />
<Switch checked={this.state.outlined} label="Outlined" onChange={this.handleOutlinedChange} />
<Switch checked={this.state.vertical} label="Vertical" onChange={this.handleVerticalChange} />
<IntentSelect intent={this.state.intent} label={intentLabelInfo} onChange={this.handleIntentChange} />
<AlignmentSelect align={this.state.alignText} onChange={this.handleAlignChange} />
Expand Down