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

Show description for boolean fields #498

Merged
merged 2 commits into from
Mar 3, 2017

Conversation

Reggino
Copy link
Contributor

@Reggino Reggino commented Feb 28, 2017

Reasons for making this change

A fix for #435 . I chose to present the description in a similar way like the other fields, with an extra text above the input. The suggested title looked a bit "strange" since it was the only input with that kind of behavior and it would probably have issues in a mobile view.

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 nits. We may want to add a small test as well.

<label>
<input type="checkbox"
id={id}
checked={typeof value === "undefined" ? false : value}
required={required}
disabled={disabled}
autoFocus={autofocus}
title={schema.description}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The title attribute should be set on the <label> element, so the tooltip is shown when hovering over the whole widget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the title-attribute completely, since it is redundant with the DescriptionField above the input.

@@ -13,13 +13,18 @@ function CheckboxWidget({
}) {
return (
<div className={`checkbox ${disabled ? "disabled" : ""}`}>
{ schema.description
? <DescriptionField description={ schema.description }/>
: null
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: this could be written as

{ schema.description && <DescriptionField description={ schema.description }/> }

@Reggino
Copy link
Contributor Author

Reggino commented Mar 3, 2017

Thanks for the feedback. Should be good to go now!

@n1k0
Copy link
Collaborator

n1k0 commented Mar 3, 2017

Thanks!

@n1k0 n1k0 merged commit 73a2ea7 into rjsf-team:master Mar 3, 2017
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.

2 participants