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

Expose disabled prop of Input #170

Merged
merged 1 commit into from Apr 7, 2019
Merged

Expose disabled prop of Input #170

merged 1 commit into from Apr 7, 2019

Conversation

tcbegley
Copy link
Collaborator

@tcbegley tcbegley commented Mar 29, 2019

This PR exposes the disabled prop of the Input component as suggested in #168

@pbugnion
Copy link
Member

pbugnion commented Apr 7, 2019

For reference, this is what the disabled state looks like:
Screenshot 2019-04-07 at 08 21 05

@@ -168,6 +168,11 @@ Input.propTypes = {
*/
value: PropTypes.string,

/**
* If true, the input is disabled and can't be clicked on.
Copy link
Member

@pbugnion pbugnion Apr 7, 2019

Choose a reason for hiding this comment

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

We're annoyingly inconsistent on using true and True in the descriptions. E.g. in_navbar on DropdownMenu has Set this to True if the dropdown is inside a navbar. Default: False., but then all disabled props use the text you suggest. As a low-priority task, we could consistently use one or the other (I think I'm somewhat more in favour of True / False since the consumers of the documentation are Python users).

Copy link
Collaborator Author

@tcbegley tcbegley Apr 7, 2019

Choose a reason for hiding this comment

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

Good point. I agree that using True / False makes the most sense.

@pbugnion
Copy link
Member

pbugnion commented Apr 7, 2019

Looks great!

@tcbegley tcbegley merged commit b70140a into master Apr 7, 2019
@tcbegley tcbegley deleted the disabled-input branch Apr 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants