Skip to content

Commit fd8a6c6

Browse files
dsmmckenmattrunyon
andauthored
fix: add right margin to <Button kind='inline'/> using icons (#1664)
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>
1 parent 1e40d3e commit fd8a6c6

50 files changed

Lines changed: 47 additions & 26 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/code-studio/src/styleguide/Buttons.tsx

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
/* eslint-disable react/jsx-props-no-spreading */
22
import React, { Component, ReactElement } from 'react';
3-
import classNames from 'classnames';
4-
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
5-
import { ButtonOld, SocketedButton } from '@deephaven/components';
3+
import { Button, ButtonOld, SocketedButton } from '@deephaven/components';
64
import { dhTruck } from '@deephaven/icons';
75
import { sampleSectionIdAndClasses } from './utils';
86

@@ -95,37 +93,36 @@ class Buttons extends Component<Record<string, never>, ButtonsState> {
9593
>
9694
<h5>Inline Buttons</h5>
9795
Regular btn-inline:
98-
<ButtonOld className="btn-inline mx-2">
99-
<FontAwesomeIcon icon={dhTruck} />
100-
</ButtonOld>
96+
<Button className="mx-2" kind="inline" icon={dhTruck} tooltip="test" />
10197
Toggle button (class active):
102-
<ButtonOld
103-
className={classNames('btn-inline mx-2', { active: toggle })}
98+
<Button
99+
className="mx-2"
104100
onClick={() => {
105101
this.setState({ toggle: !toggle });
106102
}}
107-
>
108-
<FontAwesomeIcon icon={dhTruck} />
109-
</ButtonOld>
103+
active={toggle}
104+
kind="inline"
105+
icon={dhTruck}
106+
tooltip="test"
107+
/>
110108
Disabled:
111-
<ButtonOld className="btn-inline mx-2" disabled>
112-
<FontAwesomeIcon icon={dhTruck} />
113-
</ButtonOld>
109+
<Button className="mx-2" kind="inline" disabled>
110+
Disabled
111+
</Button>
112+
With Text:
113+
<Button className="mx-2" kind="inline" icon={dhTruck}>
114+
<span>Text Button</span>
115+
</Button>
114116
<br />
115117
<br />
116118
<span>btn-link-icon (no text):</span>
117-
<ButtonOld className="btn-link btn-link-icon px-2">
118-
{/* pad and margin horizontally as appropriate for icon shape and spacing,
119-
needs btn-link and btn-link-icon classes. */}
120-
<FontAwesomeIcon icon={dhTruck} />
121-
</ButtonOld>
119+
<Button kind="ghost" icon={dhTruck} tooltip="test" />
122120
<span className="mx-2">btn-link:</span>
123-
<ButtonOld className="btn-link">Text Button</ButtonOld>
121+
<Button kind="ghost">Text Button </Button>
124122
<span className="mx-2">btn-link (text w/ optional with icon):</span>
125-
<ButtonOld className="btn-link">
126-
<FontAwesomeIcon icon={dhTruck} />
127-
Add Item
128-
</ButtonOld>
123+
<Button kind="ghost" icon={dhTruck}>
124+
Text Button
125+
</Button>
129126
</div>
130127
);
131128
}

packages/components/src/Button.tsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,30 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
126126
);
127127
}
128128

129+
// Best effort backwards-compatible attempt to auto add margin to icon if text is also present
130+
// 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
131+
if (iconElem != null && children != null) {
132+
// check if react children contains a text node to a depth of 2
133+
// to exlude poppers/menus, but not button text nested in spans
134+
let containsTextNode = false;
135+
React.Children.forEach(children, child => {
136+
if (typeof child === 'string') {
137+
containsTextNode = true;
138+
} else if (React.isValidElement(child)) {
139+
React.Children.forEach(child.props.children, grandchild => {
140+
if (typeof grandchild === 'string') {
141+
containsTextNode = true;
142+
}
143+
});
144+
}
145+
});
146+
if (containsTextNode) {
147+
iconElem = React.cloneElement(iconElem, {
148+
className: classNames('mr-1', iconElem.props.className),
149+
});
150+
}
151+
}
152+
129153
let tooltipElem: JSX.Element | undefined;
130154
if (tooltip !== undefined) {
131155
tooltipElem =
@@ -180,10 +204,10 @@ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
180204
// disabled buttons tooltips need a wrapped element to receive pointer events
181205
// https://jakearchibald.com/2017/events-and-disabled-form-fields/
182206

183-
return disabled ? (
207+
return disabled && tooltip != null ? (
184208
<span className="btn-disabled-wrapper">
185209
{button}
186-
{tooltip !== undefined && tooltipElem}
210+
{tooltipElem}
187211
</span>
188212
) : (
189213
button
2.3 KB
2.54 KB
2.28 KB
2.14 KB
2.59 KB
2.24 KB
2.14 KB
2.64 KB

0 commit comments

Comments
 (0)