Skip to content

Commit

Permalink
fix: add right margin to <Button kind='inline'/> using icons (#1664)
Browse files Browse the repository at this point in the history
btn-inline did not have a rule to add margin to svgs in the css, like
other kinds. This should do it in a backwards compatible way, and ensure
all button kinds have the correct margin. We have a bunch of css for
other kinds that would be no longer necessary, but not removing in case
we use it in a non-standard way elsewhere.

```jsx
// previously missing margin between icon and text
<Button kind="inline" icon="dhTruck">Test</Button>
```

edit: was somewhat more complicated then expected, because children
doesn't mean visible children
```jsx
// should handle this case without adding margin
<Button kind="inline" icon="dhTruck"><DropdownMenu/></Button>
```

---------

Co-authored-by: Matthew Runyon <matthewrunyon@deephaven.io>
  • Loading branch information
dsmmcken and mattrunyon authored Dec 4, 2023
1 parent 1e40d3e commit fd8a6c6
Show file tree
Hide file tree
Showing 50 changed files with 47 additions and 26 deletions.
45 changes: 21 additions & 24 deletions packages/code-studio/src/styleguide/Buttons.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/* eslint-disable react/jsx-props-no-spreading */
import React, { Component, ReactElement } from 'react';
import classNames from 'classnames';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { ButtonOld, SocketedButton } from '@deephaven/components';
import { Button, ButtonOld, SocketedButton } from '@deephaven/components';
import { dhTruck } from '@deephaven/icons';
import { sampleSectionIdAndClasses } from './utils';

Expand Down Expand Up @@ -95,37 +93,36 @@ class Buttons extends Component<Record<string, never>, ButtonsState> {
>
<h5>Inline Buttons</h5>
Regular btn-inline:
<ButtonOld className="btn-inline mx-2">
<FontAwesomeIcon icon={dhTruck} />
</ButtonOld>
<Button className="mx-2" kind="inline" icon={dhTruck} tooltip="test" />
Toggle button (class active):
<ButtonOld
className={classNames('btn-inline mx-2', { active: toggle })}
<Button
className="mx-2"
onClick={() => {
this.setState({ toggle: !toggle });
}}
>
<FontAwesomeIcon icon={dhTruck} />
</ButtonOld>
active={toggle}
kind="inline"
icon={dhTruck}
tooltip="test"
/>
Disabled:
<ButtonOld className="btn-inline mx-2" disabled>
<FontAwesomeIcon icon={dhTruck} />
</ButtonOld>
<Button className="mx-2" kind="inline" disabled>
Disabled
</Button>
With Text:
<Button className="mx-2" kind="inline" icon={dhTruck}>
<span>Text Button</span>
</Button>
<br />
<br />
<span>btn-link-icon (no text):</span>
<ButtonOld className="btn-link btn-link-icon px-2">
{/* pad and margin horizontally as appropriate for icon shape and spacing,
needs btn-link and btn-link-icon classes. */}
<FontAwesomeIcon icon={dhTruck} />
</ButtonOld>
<Button kind="ghost" icon={dhTruck} tooltip="test" />
<span className="mx-2">btn-link:</span>
<ButtonOld className="btn-link">Text Button</ButtonOld>
<Button kind="ghost">Text Button </Button>
<span className="mx-2">btn-link (text w/ optional with icon):</span>
<ButtonOld className="btn-link">
<FontAwesomeIcon icon={dhTruck} />
Add Item
</ButtonOld>
<Button kind="ghost" icon={dhTruck}>
Text Button
</Button>
</div>
);
}
Expand Down
28 changes: 26 additions & 2 deletions packages/components/src/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,30 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
);
}

// Best effort backwards-compatible attempt to auto add margin to icon if text is also present
// We would need to audit our usage of Buttons to remove margins by classname to just add the margin to every icon button with children
if (iconElem != null && children != null) {
// check if react children contains a text node to a depth of 2
// to exlude poppers/menus, but not button text nested in spans
let containsTextNode = false;
React.Children.forEach(children, child => {
if (typeof child === 'string') {
containsTextNode = true;
} else if (React.isValidElement(child)) {
React.Children.forEach(child.props.children, grandchild => {
if (typeof grandchild === 'string') {
containsTextNode = true;
}
});
}
});
if (containsTextNode) {
iconElem = React.cloneElement(iconElem, {
className: classNames('mr-1', iconElem.props.className),
});
}
}

let tooltipElem: JSX.Element | undefined;
if (tooltip !== undefined) {
tooltipElem =
Expand Down Expand Up @@ -180,10 +204,10 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
// disabled buttons tooltips need a wrapped element to receive pointer events
// https://jakearchibald.com/2017/events-and-disabled-form-fields/

return disabled ? (
return disabled && tooltip != null ? (
<span className="btn-disabled-wrapper">
{button}
{tooltip !== undefined && tooltipElem}
{tooltipElem}
</span>
) : (
button
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.
Binary file modified tests/styleguide.spec.ts-snapshots/icons-chromium-linux.png
Binary file modified tests/styleguide.spec.ts-snapshots/icons-firefox-linux.png
Binary file modified tests/styleguide.spec.ts-snapshots/icons-webkit-linux.png

0 comments on commit fd8a6c6

Please sign in to comment.