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

Changed incorrect readonly prop of Input by right one: readOnly #348

Merged
merged 1 commit into from Oct 29, 2018

Conversation

Projects
None yet
3 participants
@Akronix
Copy link
Contributor

commented Oct 26, 2018

the prop readonly of Input component is incorrect and therefore, it does not work. Replaced by correct one: readOnly.

@T4rk1n

T4rk1n approved these changes Oct 26, 2018

Copy link
Contributor

left a comment

💃

@T4rk1n T4rk1n requested a review from valentijnnieman Oct 26, 2018

@valentijnnieman
Copy link
Contributor

left a comment

Thanks for contributing! What is wrong with the readonly prop in your opinion? I think it is supposed to be lowercase because it's supposed to be an attribute to the <input> HTML tag, as here: https://www.w3schools.com/tags/att_input_readonly.asp

@T4rk1n

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

@Akronix

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2018

Exactly. As @valentijnnieman pointed out, readonly is not recognized as a DOM attribute for the React input element (here we are using React or not rendering directly to an html tag). This is because of their camelCase convention:

In React, all DOM properties and attributes (including event handlers) should be camelCased. For example, the HTML attribute tabindex corresponds to the attribute tabIndex in React. The exception is aria-* and data-* attributes, which should be lowercased. For example, you can keep aria-label as aria-label.

Source: https://reactjs.org/docs/dom-elements.html

Actually, if you try to use readonly attribute in this component (Input) it doesn't work, but it does work for the Textarea component of dash-html-components which has this attribute as readOnly. That's how I found the bug.

@T4rk1n T4rk1n merged commit aa3557e into plotly:master Oct 29, 2018

4 checks passed

ci/circleci: python-2.7 Your tests passed on CircleCI!
Details
ci/circleci: python-3.6 Your tests passed on CircleCI!
Details
ci/circleci: python-3.7 Your tests passed on CircleCI!
Details
percy/dash-core-components Visual review automatically approved, no visual changes found.
Details

@T4rk1n T4rk1n referenced this pull request Oct 29, 2018

Merged

Version bump 0.35.1 #349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.