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

feat: re-export Spectrum ButtonGroup #2028

Merged
merged 12 commits into from
May 24, 2024
61 changes: 23 additions & 38 deletions packages/code-studio/src/styleguide/Buttons.tsx
Copy link
Member

Choose a reason for hiding this comment

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

Nice

Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import React, { Component, ReactElement } from 'react';
import { Button, SocketedButton, Flex } from '@deephaven/components';
import {
Button,
SocketedButton,
Flex,
ButtonGroup,
} from '@deephaven/components';

import { dhTruck } from '@deephaven/icons';
import SampleSection from './SampleSection';
Expand All @@ -17,7 +22,7 @@ class Buttons extends Component<Record<string, never>, ButtonsState> {
<>
<h5>Button Kinds</h5>
<SampleSection name="buttons-regular" style={{ padding: '1rem 0' }}>
<Flex gap="size-100">
<ButtonGroup>
<Button kind="primary" onClick={noOp}>
Primary
</Button>
Expand All @@ -39,7 +44,7 @@ class Buttons extends Component<Record<string, never>, ButtonsState> {
<Button kind="ghost" onClick={noOp}>
Ghost
</Button>
</Flex>
</ButtonGroup>
</SampleSection>
</>
);
Expand Down Expand Up @@ -74,41 +79,21 @@ class Buttons extends Component<Record<string, never>, ButtonsState> {
return (
<SampleSection name="buttons-socketed">
<h5>Socketed Buttons (for linker)</h5>
<SocketedButton
style={{ marginBottom: '1rem', marginRight: '1rem' }}
onClick={noOp}
>
Unlinked
</SocketedButton>
<SocketedButton
style={{ marginBottom: '1rem', marginRight: '1rem' }}
isLinked
onClick={noOp}
>
Linked
</SocketedButton>
<SocketedButton
style={{ marginBottom: '1rem', marginRight: '1rem' }}
isLinkedSource
onClick={noOp}
>
Linked Source
</SocketedButton>
<SocketedButton
style={{ marginBottom: '1rem', marginRight: '1rem' }}
isLinked
isInvalid
onClick={noOp}
>
Error
</SocketedButton>
<SocketedButton
style={{ marginBottom: '1rem', marginRight: '1rem' }}
disabled
onClick={noOp}
>
Disabled
</SocketedButton>
<ButtonGroup marginBottom="1rem">
<SocketedButton onClick={noOp}>Unlinked</SocketedButton>
<SocketedButton isLinked onClick={noOp}>
Linked
</SocketedButton>
<SocketedButton isLinkedSource onClick={noOp}>
Linked Source
</SocketedButton>
<SocketedButton isLinked isInvalid onClick={noOp}>
Error
</SocketedButton>
<SocketedButton disabled onClick={noOp}>
Disabled
</SocketedButton>
</ButtonGroup>
</SampleSection>
);
}
Expand Down
16 changes: 14 additions & 2 deletions packages/components/src/Button.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { useSlotProps } from '@react-spectrum/utils';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
Expand Down Expand Up @@ -83,7 +84,7 @@ function getVariantClasses(kind: VariantKind): string {
}
}

const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
export const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
(props: ButtonProps, ref) => {
const {
kind,
Expand All @@ -109,6 +110,16 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
...rest
} = props;

// Spectrum container components such as `ButtonGroup` provide
// UNSAFE_className prop for the `button` slot via a SlotProvider (
// https://github.com/adobe/react-spectrum/blob/%40adobe/react-spectrum%403.33.1/packages/%40react-spectrum/buttongroup/src/ButtonGroup.tsx#L104-L107)
// This can be retrieved via `useSlotProps` to allow our buttons to be styled
// correctly inside the container.
const { UNSAFE_className } = useSlotProps<{ UNSAFE_className?: string }>(
{},
'button'
);

let variantClassName;
if (variant) {
variantClassName = getVariantClasses(variant);
Expand Down Expand Up @@ -157,7 +168,8 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
btnClassName,
variantClassName,
{ active },
className
className,
UNSAFE_className
)}
onClick={onClick}
onContextMenu={onContextMenu}
Expand Down
17 changes: 15 additions & 2 deletions packages/components/src/SocketedButton.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import React from 'react';
import classNames from 'classnames';
import { dhExclamation, vsLink } from '@deephaven/icons';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { useSlotProps } from '@react-spectrum/utils';
import { dhExclamation, vsLink } from '@deephaven/icons';

import './SocketedButton.scss';

Expand Down Expand Up @@ -36,6 +37,17 @@ const SocketedButton = React.forwardRef<HTMLButtonElement, SocketedButtonProps>(
style,
'data-testid': dataTestId,
} = props;

// Spectrum container components such as `ButtonGroup` provide
// UNSAFE_className prop for the `button` slot via a SlotProvider (
// https://github.com/adobe/react-spectrum/blob/%40adobe/react-spectrum%403.33.1/packages/%40react-spectrum/buttongroup/src/ButtonGroup.tsx#L104-L107)
// This can be retrieved via `useSlotProps` to allow our buttons to be styled
// correctly inside the container.
const { UNSAFE_className } = useSlotProps<{ UNSAFE_className?: string }>(
{},
'button'
);

return (
<button
ref={ref}
Expand All @@ -48,7 +60,8 @@ const SocketedButton = React.forwardRef<HTMLButtonElement, SocketedButtonProps>(
},
{ 'btn-socketed-linked-source': isLinkedSource },
{ 'is-invalid': isInvalid },
className
className,
UNSAFE_className
)}
id={id}
onClick={onClick}
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export * from './actions';
export { default as AutoCompleteInput } from './AutoCompleteInput';
export { default as AutoResizeTextarea } from './AutoResizeTextarea';
export { default as BasicModal } from './BasicModal';
export { default as Button } from './Button';
export * from './Button';
export * from './BulkActionBar';
export { default as CardFlip } from './CardFlip';
export * from './context-actions';
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/spectrum/buttons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ export {
ActionButton,
type SpectrumActionButtonProps as ActionButtonProps,
// Button - we want to use our own `Button` component instead of Spectrum's
// ButtonGroup - will re-export once our `Button` is compatible
ButtonGroup,
type SpectrumButtonGroupProps as ButtonGroupProps,
// FileTrigger - we aren't planning to support this component
LogicButton,
type SpectrumLogicButtonProps as LogicButtonProps,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,25 @@ svg[class*='spectrum-Textfield-validationIcon'] {
);
}

button[class*='spectrum-Button'] {
/**
Adjust Spectrum button text and icon alignment to account for DH using a
different font and smaller icons. Spectrum `ButtonGroup` will add additional
`-spectrum-ButtonGroup` classes to child buttons (including DH ones), so we
explicitly exclude our buttons from the selector.
*/
button[class*='spectrum-Button']:not(.btn):not(.btn-socketed) {
/* make the icon closer to the text */
--spectrum-button-primary-text-gap: var(--spectrum-global-dimension-size-75);

/* Center the text vertically. We use a different font then spectrum so we require different custom centering */
padding-bottom: calc(var(--spectrum-global-dimension-size-50) - 1px);
padding-top: calc(var(--spectrum-global-dimension-size-50));
}

button[class*='spectrum-Button'] svg {
/* our icons are smaller, center them too */
button[class*='spectrum-Button']:not(.btn):not(.btn-socketed) svg {
/*
We use smaller icons than Spectrum, so we need to adjust the padding to
center when we use our icons inside of Spectrum buttons.
*/
padding-bottom: var(--spectrum-global-dimension-size-25);
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Member

Choose a reason for hiding this comment

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

Just noticed we have the same snapshots taken a few times for the socketed buttons section - is that necessary? Seems like we should only be taking one snapshot of the section to verify everything?

In any case, not related to this change necessarily. Can be a follow up if needed to clean up.

Loading