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

Fix CheckboxesWidget and RadioWidget style #403

Merged
merged 2 commits into from
Nov 21, 2016

Conversation

maartenth
Copy link
Contributor

@maartenth maartenth commented Nov 21, 2016

Reasons for making this change

Unfortunately I currently don't have time for the 'bigger' issues, but this is a simple style fix I think will make many forms look better.

Some context:
The label in a CheckboxWidget is bold, and rightly so, because the label of a CheckboxWidget is the field title and all field titles are bold. The labels in a RadioWidget are not bold, unless the option inline: true is set.

Issue 1:
The labels in the CheckboxesWidget are bold too. I think this is done for consistency with CheckboxWidget, but I think consistency with RadioWidget would be better, because the labels of a CheckboxesWidget are not titles but enumNames. Now it could lead to an enormous wall of bold text for large enums, and the real field title does not stand out any more. I changed this in this pull request.

Issue 2:
The labels in RadioWidget and CheckboxesWidget were bold when inline: true was set. This was due to a styling issue with Bootstrap, which needs another DOM structure for inline widgets (class radio-inline or checkbox-inline must be set on <label> itself instead of on a wrapper <div>). I fixed this too.

Issue 3:
While I was working on it, I saw that in CheckboxesWidget the disabled class was not set for disabled inputs. I fixed this like it was done for CheckboxWidget and RadioWidget.

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

@n1k0
Copy link
Collaborator

n1k0 commented Nov 21, 2016

Looks good to me. Thank you :)

@n1k0 n1k0 merged commit e1738ed into rjsf-team:master Nov 21, 2016
n1k0 added a commit that referenced this pull request Nov 25, 2016
* Update dependencies to their latest versions. (#386)
* Fix #385: Avoid dynamic requires. (#387)
* Fix #365: Tab stop for Add button + (#392)
* Add a single field form example in the playground (#390)
* Allow using field names containing a dot character (#397)
* Updated eslint config.
* Improve checkbox and radio button styles (#403)
* Add missing proptype for disabled (#416)
* Temporary fix for #349 and facebook/react#7630 radio widget bug (#423)
@n1k0
Copy link
Collaborator

n1k0 commented Nov 25, 2016

Released along v0.41.2.

@maartenth maartenth deleted the fix-array-values-not-bold branch November 25, 2016 16:05
@wintercounter
Copy link
Contributor

This is great, but you removed <strong> from the markup which I used to style my checkboxes: input:checked + strong
I was happy that at least checkboxes had a wrapper for text. Please wrap the text into something so it'll be possible to use the :checked selector for both radio and checkbox. Just an empty span at least.

@maartenth
Copy link
Contributor Author

@wintercounter I'm not the one who decides but I think a <span> would not hurt anyone, you could do a PR to fix it for both CheckboxWidget, CheckboxesWidget and RadioWidget.

@n1k0
Copy link
Collaborator

n1k0 commented Dec 6, 2016 via email

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