Terra-core: Uplift Control Component for OCS

Created on 23 Oct 2017  路  14Comments  路  Source: cerner/terra-core

Issue Description

Uplift control component.

Issue Type

  • [ ] New Feature
  • [x] Enhancement
  • [ ] Bug
  • [ ] Other
terra-form-checkbox terra-form-radio

Most helpful comment

Talked in scrum this morning, lets also decouple this from the input component and create our own input with html.

All 14 comments

Control Tech Design

Summary:

The first phase of the uplift will be to create two components, radio-button and checkbox, which will encapsulate the existing control component and maintain existing functionality. Then, to add themeable variables to these new components and also control.

Responsiveness:

Value label within control will wrap if it exceeds the container's width.

Accessibility

  • A user can navigate to a control component using tab or arrow keys.
  • A user can select a radio button/checkbox by using space.

Needed React Props:

These components will have the exact same props as control, except the control prop type will be passed in as a hardcoded string for the respective control component, labelText is now required, and isLabelHidden will be added as a bool to hide the label if necessary.

|Prop Name|Type|Is Required|Default Value|Description|
|---|---|---|---|---|
| checked|bool| optional|undefined|The checked property of the Input element. Use this to generate a controlled Control Element.|
| defaultChecked|bool|optional|undefined|The checked property of the Input element. Use this to generate an uncontrolled Control Element.|
|id| string| optional|undefined|The id of the input field.|
|inputAttrs| object|optional| {}|Additional attributes for the input object.|
|isLabelHidden|bool|optional|false|Whether the label is hidden or not.|
|isInline| bool|optional|false|Whether the form element is inline or not.|
|labelText| node|required|null|Text of the label.|
|labelTextAttrs|object| optional| {}|Additional objects for the text object.|
|name| string|optional|null|Name of the input attribute.|
|onChange|func|optional|undefined|Function to trigger when user clicks on the input. Provide a function to create a controlled input.|
|value| string| optional| undefined|The value of the input element.|

Expected Future Props:

We will need props that indicate error, disabled, and required states.

|Prop Name|Type|Is Required|Description|
|---|---|---|---|
|isDisabled|bool|optional|Whether the control field is disabled or not.|

Themeable Styles

Control Themeable Styles

  • Button background
  • Button border
  • Button height
  • Button width
  • Control focus outline
  • Control disabled background
  • Control width
  • Control height
  • Label margin
  • Label hover state
  • Label font color
  • Label font family
  • Label font Size

Notes / Questions

In the future, higher order radio-button-group and checkbox-group components will need to be created that will have radio-button and checkbox as children.

  • If the groups are inline, the label margins should be applied on the right to all, except the first. This stops a wrapped control component from being indented on the next line.
  • If the groups are vertically aligned, the label margins should be applied on the left
  • These groups should default to an inline display unless the group exceeds the container's width, in which case the group will be displayed vertically.

The future prop inError will actually need to be isInvalid to be consistent with Field & TextArea.

+1

We need to make labelText required. All form fields should include a label for accessibility. Though I know UX has designs which do not display a visual label. For this case, we should add a prop, isLabelHidden which defaults for false which can be used to visually hide the label, while maintaining it for screen readers. This is a change we need to make across the board, but for any new form components, we should be including this prop change by default.

Logged https://github.com/cerner/terra-core/issues/973 to work on this issue in the other form fields.

Other than that, +1 on the tech design.

Updated labelText to be required and added isLabelHidden to props.

+1

RadioButton feels verbose. I'd get a few votes between that and Radio

My vote would be for using Radio. I did a quick look around at other UI toolkits and there doesn't seem to be a standard. Some use RadioButton and some use Radio

I would also vote Radio. With the form components, we initially tried to align with the html syntax when possible (i.e the required prop when our bool standard would have desired isRequired). Radio would align with this.

^^^To play devil's advocate, Radio is more vague, but I'm fine with refactoring the name.

Also, because we decided on splitting up form components, I'm planning to create a separate terra-form-control package containing radio and checkbox. They'll share a style sheet and pass in input and label styles through the existing props inputAttrs and labeTextAttrs. I also propose to add a new prop, controlAttrs, so radio and checkbox can pass in styling for the outer control container element as well. Thoughts?

Update

Based on discussion, Radio will exist in a separate package terra-form-radio and Checkbox within terra-form-checkbox. Both will be new components and will not wrap Control, which will be deprecated in the future.

Talked in scrum this morning, lets also decouple this from the input component and create our own input with html.

Also, because we're decoupling form packages, there's no need to avoid non-passive changes. I'm adding the future props isInvalid, isDisabled, and required into these components during this iteration, to align with the added themeable variables.

*Edit
Actually, I don't believe isInvalid is needed, that seems to be part of field. There's also no invalid attribute within the input element anyway.

I would think isInvalid would be handled by Field unless there is a specific style applied to the radio button or checkbox itself when its invalid.

What will required do for these components? I would also think this was handled by Field

After discussion, I'm removing required too. If there's a case of multiple control components within Field it would be handled by a FieldSet.

I'm splitting this issue into two separate issues for checkbox and radio button.

Was this page helpful?
0 / 5 - 0 ratings