Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor Radio family to become SelectGroup family #408

Merged
merged 19 commits into from
Nov 21, 2019
Merged

Conversation

alexpaxton
Copy link
Contributor

@alexpaxton alexpaxton commented Nov 20, 2019

Closes #403

Aiming to make SelectGroup play a little nicer with HTML5 forms by making it utilize HTML inputs under the hood so a form can parse its value. The family has been renamed to indicate that it can be used for multi-select as well as single-select.

Changes

  • Rename Radio family to SelectGroup to be more semantic
    • This change was driven by the option to use multi-select and "radio" implies single-select only
  • Rename RadioButton to SelectGroupOption
  • Change the invisible inputs within Toggle and SelectGroupOption to be uncontrolled and readonly
    • Use onClick instead of onChange because onChange on a checkbox/radio input has inconsistent behavior between browsers
  • Listen for Space key instead of Enter to toggle state of Toggle and SelectGroupOption
  • Refactor SelectGroupOption to follow a similar pattern to Toggle
    • Instead of <button /> it is now composed of <input /> and <label />
    • SelectGroupOption can be of type radio (default) or checkbox
    • SelectGroupOption is now tab focusable
  • Make id prop of SelectGroupOption required
    • Necessary for it to work properly now
  • Update styles for disabled state of SelectGroupOption to look more disabled
  • Rewrite SelectGroup documentation to explain the new component API

Screenshots

radio-single-select
radio-multiselect
radio-tabbable

Checklist

Check all that apply

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@alexpaxton alexpaxton requested review from a team and mavarius November 20, 2019 20:34
@ghost ghost requested review from drdelambre and TCL735 and removed request for a team November 20, 2019 20:34
Copy link
Collaborator

@mavarius mavarius left a comment

Choose a reason for hiding this comment

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

Radio and Checkbox should either be separate components or we should name it something more generic. A radio control by its nature should only allow a single item in a list to be selected.

CHANGELOG.md Outdated
@@ -2,6 +2,9 @@

#### 1.0.8 (Unreleased)

- [#408](https://github.com/influxdata/clockface/pull/408): Refactor `RadioButton` to include either a checkbox or radio input under the hood to enable better interactions with HTML5 forms
- [#408](https://github.com/influxdata/clockface/pull/408): Enable tab focus interactions to improve `RadioButton` accessibility
Copy link
Collaborator

Choose a reason for hiding this comment

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

awesome!

@alexpaxton
Copy link
Contributor Author

Based on feedback I made these changes:

  • Rename Radio family to SelectGroup to be more semantic
    • This change was driven by the option to use multi-select and "radio" implies single-select only
  • Rename RadioButton to SelectGroupOption
  • Change the invisible inputs within Toggle and SelectGroupOption to be uncontrolled and readonly
    • Use onClick instead of onChange because onChange on a checkbox/radio input has inconsistent behavior between browsers
  • Listen for Space key instead of Enter to toggle state of Toggle and SelectGroupOption

@alexpaxton alexpaxton changed the title Improve Radio family Refactor Radio family to become SelectGroup family Nov 20, 2019
@alexpaxton alexpaxton merged commit 1a542c5 into master Nov 21, 2019
@alexpaxton alexpaxton deleted the improve-radio branch November 21, 2019 00:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add html input element underneath RadioButton
3 participants