-
Notifications
You must be signed in to change notification settings - Fork 435
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
Conversation
style={inputStyles} | ||
type='text' | ||
value={this.state.strValue} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The <input />
is the real area to focus on in this component ^
c19425e
to
5b7559b
Compare
@@ -75,6 +77,10 @@ const propTypes = { | |||
* Deselect label and icon (optional) of button. | |||
*/ | |||
stateThree: PropTypes.object, | |||
/** | |||
* Write <code>'-1'</code> if you don't want the user to tab to the button. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is parsed as markdown. Please change to "-1"
Will SLDSButton prevent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this tedious task. A few minor comments. I'll probably bump this up to 0.5.0 at release, since I'm 100% sure some of our engineers have hacked these attributes.
onInput, | ||
pattern, | ||
placeholder, | ||
...props |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs // eslint-disable-line no-unused-vars
.
onMouseEnter: PropTypes.func, | ||
onMouseLeave: PropTypes.func, | ||
/** | ||
* Write <code>"-1"</code> if you don't want the user to tab to the button. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"-1"
@minevskiy good call -- I added |
a2716af
to
7909938
Compare
This shouldn't change any functionality so if anyone sees anything, please let me know. I went through all the components and remove the
{...props}
from the DOM nodes (buttons, inputs, svg, etc).I tried to think of all the common event handlers, attributes, and a11y stuff that might be passed down to these elements and added them manually. Please let me know what I've missed.
(Just an FYI for @tweettypography @chriscorwin @futuremint @iowillhoit)
In general, I wanted to ensure these props are passed down to
<button>
s (not inclusive list):And these props are passed down to form elements (input, checkbox - again, not all inclusive list):
This PR also addresses:
Checkbox not responding to keyboard as it should - #753
Unknown props in stateful buttons - #417