Skip to content

Remove passing "all other" props to DOM nodes #773

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

Merged
merged 5 commits into from
Nov 15, 2016
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
4 changes: 4 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ These are changes that have backwards-compatible solutions present and that comp

---

## Release 0.4.9
- Removed {...props} from DOM nodes in components to prevent non-valid ones from being passed on (ie. `<input myFunkyprop .. />`)
- If users need specific props passed onto DOM nodes, please submit a github issue.

## Release 0.4.7

**MINOR FEATURES**
Expand Down
109 changes: 79 additions & 30 deletions components/button-stateful/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import TooltipTrigger from '../popover-tooltip/trigger';

import { BUTTON_STATEFUL } from '../../utilities/constants';

const blurElement = e => e.currentTarget.blur();

const propTypes = {
/**
Expand All @@ -54,15 +53,19 @@ const propTypes = {
* If true, button/icon is white. Meant for buttons or utility icons on dark backgrounds.
*/
inverse: PropTypes.bool,
onBlur: PropTypes.func,
onClick: PropTypes.func,
onFocus: PropTypes.func,
onKeyDown: PropTypes.func,
onKeyPress: PropTypes.func,
onKeyUp: PropTypes.func,
onMouseDown: PropTypes.func,
onMouseEnter: PropTypes.func,
onMouseLeave: PropTypes.func,
/**
* If true, button scales to 100% width on small form factors.
*/
responsive: PropTypes.bool,
/**
* Write <code>'-1'</code> if you don't want the user to tab to the button.
*/
tabIndex: PropTypes.string,
/**
* Initial label and icon (optional) of button.
*/
Expand All @@ -75,6 +78,10 @@ const propTypes = {
* Deselect label and icon (optional) of button.
*/
stateThree: PropTypes.object,
/**
* Write "-1" if you don't want the user to tab to the button.
*/
tabIndex: PropTypes.string,
tooltip: PropTypes.node,
variant: PropTypes.oneOf(['base', 'neutral', 'brand', 'destructive', 'icon'])
};
Expand Down Expand Up @@ -112,6 +119,11 @@ class ButtonStateful extends TooltipTrigger {
if (!isBoolean(this.props.active)) this.setState({ active: !this.state.active });
}

handleBlur(e) {
if(this.props.onBlur) this.props.onBlur(e);
e.currentTarget.blur();
}

getClassName (active) {
return classNames(this.props.className, 'slds-button', {
'slds-button--neutral': this.props.variant !== 'icon',
Expand All @@ -124,67 +136,104 @@ class ButtonStateful extends TooltipTrigger {
}

render () {
const props = omit(this.props, ['className', 'label', 'onClick', 'type', 'active']);
const active = isBoolean(this.props.active) ? this.props.active : this.state.active;

if (this.props.variant === 'icon') {
const {
active,
assistiveText,
disabled,
iconName,
iconSize,
id,
onFocus,
onKeyDown,
onKeyPress,
onKeyUp,
onMouseDown,
onMouseEnter,
onMouseLeave,
stateOne,
stateTwo,
stateThree,
tabIndex,
variant
} = this.props;

const isActive = isBoolean(active) ? active : this.state.active;

if (variant === 'icon') {
return (
<button
onMouseLeave={blurElement}
className={this.getClassName(active)}
onClick={this.handleClick.bind(this)}
{...props}
aria-live="polite"
className={this.getClassName(isActive)}
disabled={disabled}
id={id}
onBlur={this.handleBlur.bind(this)}
onClick={this.handleClick.bind(this)}
onFocus={onFocus}
onKeyDown={onKeyDown}
onKeyPress={onKeyPress}
onKeyUp={onKeyUp}
onMouseDown={onMouseDown}
onMouseEnter={onMouseEnter}
onMouseLeave={this.handleBlur.bind(this)}
tabIndex={tabIndex}
>
<ButtonIcon
assistiveText={active ? `${this.props.assistiveText} selected` : this.props.assistiveText}
disabled={this.props.disabled}
name={this.props.iconName}
size={this.props.iconSize}
assistiveText={isActive ? `${assistiveText} selected` : assistiveText}
disabled={disabled}
name={iconName}
size={iconSize}
className="slds-button__icon--stateful"
/>
{this.getTooltip()}
</button>
);
}

return (
<button
onMouseLeave={blurElement}
className={this.getClassName(active)}
aria-live="assertive"
className={this.getClassName(isActive)}
disabled={disabled}
id={id}
onBlur={this.handleBlur.bind(this)}
onClick={this.handleClick.bind(this)}
{...props}
onFocus={onFocus}
onKeyDown={onKeyDown}
onKeyPress={onKeyPress}
onKeyUp={onKeyUp}
onMouseEnter={onMouseEnter}
onMouseLeave={this.handleBlur.bind(this)}
tabIndex={tabIndex}
>
<span className="slds-text-not-selected">
<ButtonIcon
disabled={this.props.disabled}
name={this.props.stateOne.iconName}
disabled={disabled}
name={stateOne.iconName}
size="small"
position="left"
className="slds-button__icon--stateful"
/>
{this.props.stateOne.label}
{stateOne.label}
</span>
<span className="slds-text-selected">
<ButtonIcon
disabled={this.props.disabled}
name={this.props.stateTwo.iconName}
disabled={disabled}
name={stateTwo.iconName}
size="small"
position="left"
className="slds-button__icon--stateful"
/>
{this.props.stateTwo.label}
{stateTwo.label}
</span>
<span className="slds-text-selected-focus">
<ButtonIcon
disabled={this.props.disabled}
name={this.props.stateThree.iconName}
disabled={disabled}
name={stateThree.iconName}
size="small"
position="left"
className="slds-button__icon--stateful"
/>
{this.props.stateThree.label}
{stateThree.label}
</span>
{this.getTooltip()}
</button>
Expand Down
77 changes: 54 additions & 23 deletions components/button/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ import { BUTTON } from '../../utilities/constants';

const displayName = BUTTON;
const propTypes = {
/**
* Used if the Button triggers a menu or popup. Bool indicates if the menu or popup is open or closed.
*/
ariaExpanded: PropTypes.bool,
/**
* True if Button triggers a menu or popup to open/close.
*/
ariaHaspopup: PropTypes.bool,
/**
* Text that is visually hidden but read aloud by screenreaders to tell the user what the icon means.
* If the button has an icon and a visible label, you can omit the <code>assistiveText</code> prop and use the <code>label</code> prop.
Expand Down Expand Up @@ -57,7 +65,7 @@ const propTypes = {
*/
iconVariant: PropTypes.oneOf(['bare', 'container', 'border', 'border-filled', 'more', 'global-header']),
/**
* For icon variants, please reference <a href="http://www.lightningdesignsystem.com/components/buttons/#icon">Lightning Design System Icons</a>.
* Id string applied to button node.
*/
id: PropTypes.string,
/**
Expand All @@ -68,10 +76,18 @@ const propTypes = {
* Visible label on the button. If the button is an icon button with no label, you must use the <code>assistiveText</code> prop.
*/
label: PropTypes.string,
onBlur: PropTypes.func,
/**
* Triggered when the button is clicked.
*/
onClick: PropTypes.func,
onFocus: PropTypes.func,
onKeyDown: PropTypes.func,
onKeyPress: PropTypes.func,
onKeyUp: PropTypes.func,
onMouseDown: PropTypes.func,
onMouseEnter: PropTypes.func,
onMouseLeave: PropTypes.func,
/**
* If true, button scales to 100% width on small form factors.
*/
Expand Down Expand Up @@ -185,38 +201,53 @@ class Button extends TooltipTrigger {
}

render () {
const props = omit(this.props, [
'assistiveText',
'className',
'hint',
'iconCategory',
'iconName',
'iconPosition',
'iconSize',
'iconVariant',
'inverse',
'label',
'onClick',
'responsive',
'tooltip',
'variant'
]);
const {
ariaExpanded,
ariaHaspopup,
children,
disabled,
iconName,
iconPosition,
iconVariant,
id,
onBlur,
onFocus,
onKeyDown,
onKeyPress,
onKeyUp,
onMouseDown,
onMouseEnter,
onMouseLeave,
tabIndex
} = this.props;

return (
<button
aria-expanded={ariaExpanded}
aria-haspopup={ariaHaspopup}
className={this.getClassName()}
{...props}
disabled={disabled}
id={id}
onBlur={onBlur}
onClick={this.handleClick}
onFocus={onFocus}
onKeyDown={onKeyDown}
onKeyPress={onKeyPress}
onKeyUp={onKeyUp}
onMouseDown={onMouseDown}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
tabIndex={tabIndex}
>
{this.props.iconPosition === 'right' ? this.renderLabel() : null}
{iconPosition === 'right' ? this.renderLabel() : null}

{this.props.iconName ? this.renderIcon(this.props.iconName) : null}
{this.props.iconVariant === 'more'
{iconName ? this.renderIcon(this.props.iconName) : null}
{iconVariant === 'more'
? <ButtonIcon category="utility" name="down" size="x-small" />
: null}

{(this.props.iconPosition === 'left' || !this.props.iconPosition) ? this.renderLabel() : null}
{this.props.children}
{(iconPosition === 'left' || !iconPosition) ? this.renderLabel() : null}
{children}
{this.getTooltip()}
</button>
);
Expand Down
4 changes: 2 additions & 2 deletions components/card/filter/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
import React from 'react';

import Input from '../../forms/input';
import InputIcon from '../../icon/input-icon';

// Removes the need for `PropTypes`.
const { PropTypes } = React;
Expand All @@ -35,8 +36,7 @@ const Filter = (props) => {
<Input
{...rest}
assistiveText={placeholder}
iconCategory="utility"
iconName="search"
iconLeft={<InputIcon name="search" category="utility" />}
id={id}
onChange={onChange}
placeholder={placeholder}
Expand Down
Loading