Label documentation and consistency updates (#1497)

* Allow named label children as well as render results for the only child scenario
* Remove redundant README content for labels

src/checkbox-group/index.tsx

@@ -27,6 +27,7 @@ export interface CheckboxGroupChildren {
 		middleware: ReturnType<ReturnType<typeof checkboxGroup>['api']>,
 		options: CheckboxOptions
 	): RenderResult;
+	/** A label for the checkbox group */
 	label?: RenderResult;
 }
 

src/checkbox/README.md

@@ -7,10 +7,7 @@ Dojo's `Checkbox` widget provides a wrapped, styleable checkbox widget that uses
 - Creates a normal checkbox.
 - Correctly handles a11y attributes.
 - Wraps the input in a visible or invisible but accessible `<label>`.
-- Renders child content within a `label`.
 
 ### Accessibility Features
 
 `Checkbox` ensures that the proper attributes (ARIA or otherwise) are set along with classes when properties such as `disabled`, `readOnly`, `invalid`, etc. are used.
-
-If child content is not passed, a `label` will not be created. If you wish to handle this yourself, we recommend creating a separate `label` and pointing it at the input's `widgetId` property.

src/chip-typeahead/README.md

@@ -7,6 +7,7 @@ Dojo's `ChipTypeahead` widget allows users to select multiple values from a drop
 - Item and selection display can be customized
 - Selection can appear inline or below
 
+
 ### Accessiblity Features
 
 - Composes `Typeahead` and thus inherits all of its accessibility features.

src/common/util.ts

@@ -1,3 +1,6 @@
+import { DNode, RenderResult } from '@dojo/framework/core/interfaces';
+import { isVNode, isWNode } from '@dojo/framework/core/vdom';
+
 interface AriaPropertyObject {
 	[key: string]: string | null;
 }
@@ -24,3 +27,17 @@ export function formatAriaProperties(aria: AriaPropertyObject): AriaPropertyObje
 	}, {});
 	return formattedAria;
 }
+
+export function isRenderResult<T extends {}>(child: RenderResult | T): child is RenderResult {
+	let childIsRenderResult =
+		child == null ||
+		typeof child === 'string' ||
+		typeof child === 'boolean' ||
+		Array.isArray(child) ||
+		isWNode(child);
+	try {
+		childIsRenderResult = childIsRenderResult || isVNode(child as DNode);
+	} catch {}
+
+	return childIsRenderResult;
+}

src/constrained-input/README.md

@@ -20,4 +20,3 @@ A number of validation options exist for validating common username / password s
 | `contains.numbers`           | The input value must contain at least `numbers` number of numeric characters. |
 | `contains.specialCharacters` | The input value must contain at least `specialCharacters` number of special characters. |
 | `contains.atLeast`           | The input value must match `atLeast` number of the `uppercase`, `numbers`, and `specialCharacters` rules. |
-

src/date-input/index.tsx

@@ -5,7 +5,7 @@ import { focus } from '@dojo/framework/core/middleware/focus';
 import { FocusProperties } from '@dojo/framework/core/mixins/Focus';
 
 import { parseDate, formatDateISO, formatDate } from './date-utils';
-import { Keys } from '../common/util';
+import { isRenderResult, Keys } from '../common/util';
 import theme, { ThemeProperties } from '../middleware/theme';
 import Calendar from '../calendar';
 import TextInput, { Addon } from '../text-input';
@@ -15,6 +15,7 @@ import * as textInputCss from '../theme/default/text-input.m.css';
 import * as css from '../theme/default/date-input.m.css';
 
 import bundle from './nls/DateInput';
+import { RenderResult } from '@dojo/framework/core/interfaces';
 
 export interface DateInputProperties extends ThemeProperties, FocusProperties {
 	/** The initial value */
@@ -33,6 +34,11 @@ export interface DateInputProperties extends ThemeProperties, FocusProperties {
 	onValidate?: (valid: boolean | undefined, message: string) => void;
 }
 
+export interface DateInputChildren {
+	/** The label for the wrapped text input */
+	label?: RenderResult;
+}
+
 interface DateInputICache {
 	/** The most recent "initialValue" property passed */
 	initialValue: string;
@@ -53,9 +59,15 @@ interface DateInputICache {
 }
 
 const icache = createICacheMiddleware<DateInputICache>();
-const factory = create({ theme, icache, i18n, focus }).properties<DateInputProperties>();
-
-export default factory(function({ properties, middleware: { theme, icache, i18n, focus } }) {
+const factory = create({ theme, icache, i18n, focus })
+	.properties<DateInputProperties>()
+	.children<DateInputChildren | RenderResult | undefined>();
+
+export default factory(function({
+	properties,
+	children,
+	middleware: { theme, icache, i18n, focus }
+}) {
 	const { initialValue, name, onValue, onValidate, value: controlledValue } = properties();
 	const { messages } = i18n.localize(bundle);
 	const classes = theme.classes(css);
@@ -93,6 +105,8 @@ export default factory(function({ properties, middleware: { theme, icache, i18n,
 	const shouldValidate = icache.getOrSet('shouldValidate', true);
 	const shouldFocus = focus.shouldFocus();
 	const focusNode = icache.getOrSet('focusNode', 'input');
+	const [labelChild] = children();
+	const label = isRenderResult(labelChild) ? labelChild : labelChild.label;
 
 	if (shouldValidate) {
 		const testValue = icache.get('nextValue') || inputValue;
@@ -183,6 +197,7 @@ export default factory(function({ properties, middleware: { theme, icache, i18n,
 									}}
 								>
 									{{
+										label,
 										trailing: (
 											<Addon>
 												<button

src/date-input/tests/unit/DateInput.spec.tsx

@@ -1,3 +1,5 @@
+import { RenderResult, WNode } from '@dojo/framework/core/interfaces';
+
 const { describe, it, afterEach } = intern.getInterface('bdd');
 const { assert } = intern.getPlugin('chai');
 
@@ -161,6 +163,69 @@ describe('DateInput', () => {
 		sinon.assert.calledOnce(toggleOpen);
 	});
 
+	it('renders with a label as the only child', () => {
+		const h = harness(() => <DateInput name="dateInput">Label</DateInput>);
+
+		// Execute render-prop to show "trigger" content
+		const triggerResult = h.trigger(
+			'@popup',
+			(node) => (node.children as any)[0].trigger,
+			() => {}
+		);
+		h.expect(buttonTemplate, () => triggerResult);
+		assert.equal(
+			h.trigger('@popup', (node) => () =>
+				(node.children as any)[0].trigger().children[0].children[0].label
+			),
+			'Label'
+		);
+	});
+
+	it('renders with a named label child', () => {
+		const h = harness(() => <DateInput name="dateInput">{{ label: 'Label' }}</DateInput>);
+
+		// Execute render-prop to show "trigger" content
+		const triggerResult = h.trigger(
+			'@popup',
+			(node) => (node.children as any)[0].trigger,
+			() => {}
+		);
+		h.expect(buttonTemplate, () => triggerResult);
+		assert.equal(
+			h.trigger('@popup', (node) => () =>
+				(node.children as any)[0].trigger().children[0].children[0].label
+			),
+			'Label'
+		);
+	});
+
+	it('label interface edge case', () => {
+		type WeirdNode = WNode & { label: RenderResult };
+
+		function makeWeirdNode(): WeirdNode {
+			const node = <div>The label</div>;
+			(node as WeirdNode).label = 'Not the label';
+
+			return node as WeirdNode;
+		}
+		const myWeirdNode: WeirdNode = makeWeirdNode();
+		const h = harness(() => <DateInput name="dateInput">{myWeirdNode}</DateInput>);
+
+		// Execute render-prop to show "trigger" content
+		const triggerResult = h.trigger(
+			'@popup',
+			(node) => (node.children as any)[0].trigger,
+			() => {}
+		);
+		h.expect(buttonTemplate, () => triggerResult);
+		assert.equal(
+			h.trigger('@popup', (node) => () =>
+				(node.children as any)[0].trigger().children[0].children[0].label.children[0]
+			),
+			'The label'
+		);
+	});
+
 	it('shows calendar when triggered via keyboard', () => {
 		const h = harness(() => <DateInput name="dateInput" />);
 

src/native-select/index.tsx

@@ -2,14 +2,15 @@ import focus from '@dojo/framework/core/middleware/focus';
 import { i18n } from '@dojo/framework/core/middleware/i18n';
 import { createICacheMiddleware } from '@dojo/framework/core/middleware/icache';
 import { create, tsx } from '@dojo/framework/core/vdom';
+import { RenderResult } from '@dojo/framework/core/interfaces';
 import HelperText from '../helper-text';
 import theme from '../middleware/theme';
+import { isRenderResult } from '../common/util';
 import * as css from '../theme/default/native-select.m.css';
 import * as labelCss from '../theme/default/label.m.css';
 import * as iconCss from '../theme/default/icon.m.css';
 import Icon from '../icon';
 import Label from '../label';
-import { RenderResult } from '@dojo/framework/core/interfaces';
 
 export type MenuOption = { value: string; label?: string; disabled?: boolean };
 
@@ -38,6 +39,11 @@ export interface NativeSelectProperties {
 	onFocus?(): void;
 }
 
+export interface NativeSelectChildren {
+	/** The label to be displayed on the select */
+	label?: RenderResult;
+}
+
 interface NativeSelectICache {
 	initial: string;
 	value: string;
@@ -48,7 +54,7 @@ const icache = createICacheMiddleware<NativeSelectICache>();
 
 const factory = create({ icache, focus, theme, i18n })
 	.properties<NativeSelectProperties>()
-	.children<RenderResult | undefined>();
+	.children<NativeSelectChildren | RenderResult | undefined>();
 
 export const NativeSelect = factory(function NativeSelect({
 	properties,
@@ -70,7 +76,8 @@ export const NativeSelect = factory(function NativeSelect({
 		onBlur
 	} = properties();
 
-	const [label] = children();
+	const [labelChild] = children();
+	const label = isRenderResult(labelChild) ? labelChild : labelChild.label;
 	let { value } = properties();
 
 	if (value === undefined) {

src/native-select/tests/NativeSelect.spec.tsx

@@ -119,6 +119,58 @@ describe('Native Select', () => {
 		h.expect(optionalPropertyTemplate);
 	});
 
+	it('takes a named label child', () => {
+		const h = harness(
+			() => (
+				<NativeSelect
+					onValue={() => {}}
+					options={options}
+					initialValue="cat"
+					disabled={true}
+					helperText="Pick a pet type"
+					required={true}
+					name="Pet select"
+					size={3}
+				>
+					{{ label: 'Pets' }}
+				</NativeSelect>
+			),
+			[compareForId, compareId]
+		);
+
+		const optionalPropertyTemplate = baseTemplate
+			.setProperty('@native-select', 'disabled', true)
+			.setProperty('@native-select', 'required', true)
+			.setProperty('@native-select', 'name', 'Pet select')
+			.setProperty('@native-select', 'size', 3)
+			.setProperty('@helperText', 'text', 'Pick a pet type')
+			.setProperty('@option-1', 'selected', true)
+			.setProperty('@root', 'classes', [
+				undefined,
+				css.root,
+				css.disabled,
+				css.required,
+				undefined
+			])
+			.prepend('@root', () => [
+				<Label
+					assertion-key="label"
+					theme={{}}
+					classes={undefined}
+					disabled={true}
+					forId={'something'}
+					required={true}
+					active={true}
+					focused={false}
+				>
+					Pets
+				</Label>
+			])
+			.remove('@blank-option');
+
+		h.expect(optionalPropertyTemplate);
+	});
+
 	it('controlled value', () => {
 		const h = harness(
 			() => (

src/number-input/README.md

@@ -5,14 +5,10 @@ Dojo's `NumberInput` widget provides a basic number input widget with an optiona
 ## Features
 
 - Correctly handles a11y attributes
-- Associates an accessible `<label>` with the input when the `label` property is added
 - Allows leading / trailing icons / text to be added
 
 ### Accessibility Features
 
 - Ensures that the proper attributes (ARIA or otherwise) are set along with classes when properties such as `disabled`, `readOnly`, etc. are used
 - Additional custom ARIA labels may be added with the `aria` property
 - Sets `aria-invalid` when validation fails
-
-If the `label` property is not used, we recommend creating a separate `label` and pointing it at the input's `widgetId` property.
-

src/radio/README.md

@@ -7,10 +7,7 @@ Dojo's `Radio` widget provides a wrapped, styleable radio widget that uses the n
 - Creates a normal radio input.
 - Correctly handles a11y attributes.
 - Wraps the input in a visible or invisible but accessible `<label>`.
-- Renders child content within a `label`.
 
 ### Accessibility Features
 
 `Radio` ensures that the proper attributes (ARIA or otherwise) are set along with classes when properties such as `disabled`, `readOnly`, `invalid`, etc. are used.
-
-If child content is not passed, a `label` will not be created. If you wish to handle this yourself, we recommend creating a separate `label` and pointing it at the input's `widgetId` property.

src/radio/index.tsx

@@ -1,12 +1,12 @@
 import { focus } from '@dojo/framework/core/middleware/focus';
 import { FocusProperties } from '@dojo/framework/core/mixins/Focus';
 import { create, tsx } from '@dojo/framework/core/vdom';
+import { RenderResult } from '@dojo/framework/core/interfaces';
 
-import { formatAriaProperties } from '../common/util';
+import { formatAriaProperties, isRenderResult } from '../common/util';
 import theme, { ThemeProperties } from '../middleware/theme';
 import * as css from '../theme/default/radio.m.css';
 import Label from '../label';
-import { RenderResult } from '@dojo/framework/core/interfaces';
 
 export interface RadioProperties extends ThemeProperties, FocusProperties {
 	/** Custom aria attributes */
@@ -41,9 +41,14 @@ export interface RadioProperties extends ThemeProperties, FocusProperties {
 	widgetId?: string;
 }
 
+export interface RadioChildren {
+	/** The label for the radio */
+	label: RenderResult;
+}
+
 const factory = create({ focus, theme })
 	.properties<RadioProperties>()
-	.children<RenderResult | undefined>();
+	.children<RadioChildren | RenderResult | undefined>();
 
 export const Radio = factory(function Radio({
 	properties,
@@ -73,6 +78,8 @@ export const Radio = factory(function Radio({
 
 	const themeCss = theme.classes(css);
 	const idBase = widgetId || `radio-${id}`;
+	const [labelChildren] = children();
+	const label = isRenderResult(labelChildren) ? labelChildren : labelChildren.label;
 
 	return (
 		<div
@@ -133,7 +140,7 @@ export const Radio = factory(function Radio({
 					required={required}
 					secondary={true}
 				>
-					{children()}
+					{label}
 				</Label>
 			)}
 		</div>

src/radio/tests/unit/Radio.spec.tsx

@@ -138,6 +138,19 @@ registerSuite('Radio', {
 			h.expect(expected({ label: true }));
 		},
 
+		'named child label'() {
+			const h = harness(
+				() => (
+					<Radio checked={false} onValue={noop}>
+						{{ label: 'foo' }}
+					</Radio>
+				),
+				[compareId, compareForId]
+			);
+
+			h.expect(expected({ label: true }));
+		},
+
 		'state classes'() {
 			let valid = false;
 			let disabled = true;