Skip to content

Conversation

NessunKim
Copy link
Contributor

@NessunKim NessunKim commented Jan 24, 2017

Reasons for making this change

Add support for rows attribute of textarea widget.
Now users can specify initial height of it.

Checklist

  • I'm updating documentation
    • I've checked the rendering of the Markdown text I've added
    • If I'm adding a new section, I've updated the Table of Content
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

Copy link
Collaborator

@n1k0 n1k0 left a comment

Choose a reason for hiding this comment

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

LGTM with small nits.

id: PropTypes.string.isRequired,
placeholder: PropTypes.string,
options: PropTypes.shape({
enumOptions: PropTypes.bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't support enumOptions for textareas.

disabled={disabled}
readOnly={readonly}
autoFocus={autofocus}
rows={options.rows}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to ensure that options is always an object before accessing this property. We may want to define {} a default value for the options arg.

options: PropTypes.shape({
enumOptions: PropTypes.bool,
rows: PropTypes.number
}).isRequired,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please ensure that this is always actually required (I forgot about it).

@n1k0 n1k0 mentioned this pull request Jan 28, 2017
@n1k0
Copy link
Collaborator

n1k0 commented Jan 28, 2017

Poke.

@elisechant
Copy link
Contributor

@NessunKim these changes are important so we can make a release.

README.md Outdated
const schema = {type: "string"};
const uiSchema = {
"ui:widget": "textarea",
"ui:rows": 15
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm sorry, somehow I missed you weren't using the ui:options uiSchema directive, which is designed to hold this kind of widget parameters. Could you please update your patch so we use it instead?

Sorry, both for the delay and the added trouble.

Copy link
Collaborator

@n1k0 n1k0 left a comment

Choose a reason for hiding this comment

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

LGTM

@n1k0 n1k0 merged commit 3985522 into rjsf-team:master Feb 23, 2017
@NessunKim NessunKim deleted the dev branch February 23, 2017 09:44
n1k0 added a commit that referenced this pull request Mar 21, 2017
New features

* Add support for rows attribute of textarea widget. (#450)
* #434 - Render empty array item fields when minItems is specified (#484)
* Add a "has-danger" class to the form error list (#502)
* Show description for boolean fields (#498)
* Fix #488: Add a custom Form ErrorList prop.

Bugfixes

* Fix impossibility to use stateful ArrayFieldTeplate comp. (#519)
* Centralized shouldComponentUpdate handling in SchemaField (#490)
@n1k0
Copy link
Collaborator

n1k0 commented Mar 21, 2017

Released in v0.44.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants