From 093847a51b402a02413600ae603f9a56e1ad00eb Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Tue, 28 Jul 2020 20:07:07 +0100 Subject: [PATCH 01/13] Define a placeholder onPress function --- packages/@react-aria/select/src/useSelect.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index a99ecb85625..411cde8fe25 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -85,7 +85,10 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, let valueId = useId(); return { - labelProps, + labelProps: { + ...labelProps, + onPress: () => { console.log('Test') } + }, triggerProps: mergeProps(domProps, { ...triggerProps, 'aria-labelledby': [ From 2b38b821833fbb083450dfd2c72f38e9dbd5db27 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Tue, 28 Jul 2020 20:07:34 +0100 Subject: [PATCH 02/13] Pass onPress through label --- packages/@react-spectrum/label/src/Label.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/@react-spectrum/label/src/Label.tsx b/packages/@react-spectrum/label/src/Label.tsx index a816755beec..d794080d8d2 100644 --- a/packages/@react-spectrum/label/src/Label.tsx +++ b/packages/@react-spectrum/label/src/Label.tsx @@ -21,6 +21,7 @@ import {SpectrumLabelProps} from '@react-types/label'; import styles from '@adobe/spectrum-css-temp/components/fieldlabel/vars.css'; import {useMessageFormatter} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; +import {usePress} from '@react-aria/interactions'; function Label(props: SpectrumLabelProps, ref: DOMRef) { props = useProviderProps(props); @@ -34,6 +35,7 @@ function Label(props: SpectrumLabelProps, ref: DOMRef) { htmlFor, for: labelFor, elementType: ElementType = 'label', + onPress, ...otherProps } = props; @@ -59,10 +61,13 @@ function Label(props: SpectrumLabelProps, ref: DOMRef) { styleProps.className ); + const {pressProps} = usePress({onPress, ref: domRef}) + return ( From 8e7b647720635d469f1e1ab1a74a329b4e2cf469 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Tue, 28 Jul 2020 20:41:12 +0100 Subject: [PATCH 03/13] Fix press event types --- packages/@react-aria/select/src/useSelect.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 411cde8fe25..8e5cab60cae 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -14,7 +14,7 @@ import {AriaButtonProps} from '@react-types/button'; import {AriaSelectProps} from '@react-types/select'; import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; import {HTMLAttributes, RefObject, useMemo} from 'react'; -import {KeyboardDelegate} from '@react-types/shared'; +import {KeyboardDelegate, PressEvents} from '@react-types/shared'; import {ListKeyboardDelegate, useTypeSelect} from '@react-aria/selection'; import {SelectState} from '@react-stately/select'; import {useCollator} from '@react-aria/i18n'; @@ -29,9 +29,10 @@ interface AriaSelectOptions extends AriaSelectProps { keyboardDelegate?: KeyboardDelegate } +interface LabelProps extends HTMLAttributes, PressEvents {} interface SelectAria { /** Props for the label element. */ - labelProps: HTMLAttributes, + labelProps: LabelProps, /** Props for the popup trigger element. */ triggerProps: AriaButtonProps, @@ -87,7 +88,7 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, return { labelProps: { ...labelProps, - onPress: () => { console.log('Test') } + onPress: () => ref.current.focus() }, triggerProps: mergeProps(domProps, { ...triggerProps, From ae5dfafaecc8cf14284d11b247f7991cd80cd161 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Tue, 28 Jul 2020 21:06:02 +0100 Subject: [PATCH 04/13] usePress in useSelect --- packages/@react-aria/select/src/useSelect.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 8e5cab60cae..2c8abe21f8a 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -20,6 +20,7 @@ import {SelectState} from '@react-stately/select'; import {useCollator} from '@react-aria/i18n'; import {useLabel} from '@react-aria/label'; import {useMenuTrigger} from '@react-aria/menu'; +import { usePress } from '@react-aria/interactions'; interface AriaSelectOptions extends AriaSelectProps { /** @@ -85,10 +86,12 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, let triggerProps = mergeProps(mergeProps(menuTriggerProps, fieldProps), typeSelectProps); let valueId = useId(); + const {pressProps} = usePress({onPress: () => ref.current.focus()}) + return { labelProps: { ...labelProps, - onPress: () => ref.current.focus() + ...pressProps }, triggerProps: mergeProps(domProps, { ...triggerProps, From 2a44fd34ff7c0c9eed3f69ec68c93241edbf5394 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Tue, 28 Jul 2020 21:06:24 +0100 Subject: [PATCH 05/13] Pass props through Label --- packages/@react-spectrum/label/src/Label.tsx | 12 ++++++++---- packages/@react-types/label/src/index.d.ts | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/@react-spectrum/label/src/Label.tsx b/packages/@react-spectrum/label/src/Label.tsx index d794080d8d2..9884a346d9b 100644 --- a/packages/@react-spectrum/label/src/Label.tsx +++ b/packages/@react-spectrum/label/src/Label.tsx @@ -21,7 +21,6 @@ import {SpectrumLabelProps} from '@react-types/label'; import styles from '@adobe/spectrum-css-temp/components/fieldlabel/vars.css'; import {useMessageFormatter} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; -import {usePress} from '@react-aria/interactions'; function Label(props: SpectrumLabelProps, ref: DOMRef) { props = useProviderProps(props); @@ -35,7 +34,12 @@ function Label(props: SpectrumLabelProps, ref: DOMRef) { htmlFor, for: labelFor, elementType: ElementType = 'label', - onPress, + onClick, + onKeyDown, + onKeyUp, + onMouseDown, + onPointerDown, + onPointerUp, ...otherProps } = props; @@ -61,13 +65,13 @@ function Label(props: SpectrumLabelProps, ref: DOMRef) { styleProps.className ); - const {pressProps} = usePress({onPress, ref: domRef}) + const pressProps = {onClick, onKeyDown, onKeyUp, onMouseDown , onPointerDown, onPointerUp} return ( diff --git a/packages/@react-types/label/src/index.d.ts b/packages/@react-types/label/src/index.d.ts index 72bc5b7020b..ff614905fd3 100644 --- a/packages/@react-types/label/src/index.d.ts +++ b/packages/@react-types/label/src/index.d.ts @@ -10,8 +10,8 @@ * governing permissions and limitations under the License. */ -import {Alignment, DOMProps, LabelPosition, NecessityIndicator, StyleProps} from '@react-types/shared'; -import {ElementType, ReactNode} from 'react'; +import {Alignment, DOMProps, LabelPosition, NecessityIndicator, StyleProps, PressEvents} from '@react-types/shared'; +import {ElementType, ReactNode, HTMLAttributes} from 'react'; export interface LabelProps { children?: ReactNode, @@ -20,7 +20,7 @@ export interface LabelProps { elementType?: ElementType } -export interface SpectrumLabelProps extends LabelProps, DOMProps, StyleProps { +export interface SpectrumLabelProps extends LabelProps, DOMProps, StyleProps, HTMLAttributes { labelPosition?: LabelPosition, // default top labelAlign?: Alignment, // default start isRequired?: boolean, From 58876bcf3e16be81618db8caf8b0a38b48b1929f Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Tue, 28 Jul 2020 21:15:40 +0100 Subject: [PATCH 06/13] Linting fixes --- packages/@react-aria/select/src/useSelect.ts | 4 ++-- packages/@react-spectrum/label/src/Label.tsx | 2 +- packages/@react-types/label/src/index.d.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 2c8abe21f8a..4848da84a10 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -20,7 +20,7 @@ import {SelectState} from '@react-stately/select'; import {useCollator} from '@react-aria/i18n'; import {useLabel} from '@react-aria/label'; import {useMenuTrigger} from '@react-aria/menu'; -import { usePress } from '@react-aria/interactions'; +import {usePress} from '@react-aria/interactions'; interface AriaSelectOptions extends AriaSelectProps { /** @@ -86,7 +86,7 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, let triggerProps = mergeProps(mergeProps(menuTriggerProps, fieldProps), typeSelectProps); let valueId = useId(); - const {pressProps} = usePress({onPress: () => ref.current.focus()}) + const {pressProps} = usePress({onPress: () => ref.current.focus()}); return { labelProps: { diff --git a/packages/@react-spectrum/label/src/Label.tsx b/packages/@react-spectrum/label/src/Label.tsx index 9884a346d9b..203dd4d0178 100644 --- a/packages/@react-spectrum/label/src/Label.tsx +++ b/packages/@react-spectrum/label/src/Label.tsx @@ -65,7 +65,7 @@ function Label(props: SpectrumLabelProps, ref: DOMRef) { styleProps.className ); - const pressProps = {onClick, onKeyDown, onKeyUp, onMouseDown , onPointerDown, onPointerUp} + const pressProps = {onClick, onKeyDown, onKeyUp, onMouseDown, onPointerDown, onPointerUp}; return ( Date: Fri, 31 Jul 2020 15:01:50 +0100 Subject: [PATCH 07/13] Simplify label clicking --- packages/@react-aria/select/src/useSelect.ts | 11 +++++------ packages/@react-spectrum/label/src/Label.tsx | 9 +-------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 4848da84a10..989940f7fcc 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -20,7 +20,6 @@ import {SelectState} from '@react-stately/select'; import {useCollator} from '@react-aria/i18n'; import {useLabel} from '@react-aria/label'; import {useMenuTrigger} from '@react-aria/menu'; -import {usePress} from '@react-aria/interactions'; interface AriaSelectOptions extends AriaSelectProps { /** @@ -30,10 +29,9 @@ interface AriaSelectOptions extends AriaSelectProps { keyboardDelegate?: KeyboardDelegate } -interface LabelProps extends HTMLAttributes, PressEvents {} interface SelectAria { /** Props for the label element. */ - labelProps: LabelProps, + labelProps: HTMLAttributes, /** Props for the popup trigger element. */ triggerProps: AriaButtonProps, @@ -86,12 +84,13 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, let triggerProps = mergeProps(mergeProps(menuTriggerProps, fieldProps), typeSelectProps); let valueId = useId(); - const {pressProps} = usePress({onPress: () => ref.current.focus()}); - return { labelProps: { ...labelProps, - ...pressProps + onClick: () => { + const button = ref.current; + if(!button.disabled) ref.current.focus() + }, }, triggerProps: mergeProps(domProps, { ...triggerProps, diff --git a/packages/@react-spectrum/label/src/Label.tsx b/packages/@react-spectrum/label/src/Label.tsx index 203dd4d0178..bdf077249e4 100644 --- a/packages/@react-spectrum/label/src/Label.tsx +++ b/packages/@react-spectrum/label/src/Label.tsx @@ -35,11 +35,6 @@ function Label(props: SpectrumLabelProps, ref: DOMRef) { for: labelFor, elementType: ElementType = 'label', onClick, - onKeyDown, - onKeyUp, - onMouseDown, - onPointerDown, - onPointerUp, ...otherProps } = props; @@ -65,13 +60,11 @@ function Label(props: SpectrumLabelProps, ref: DOMRef) { styleProps.className ); - const pressProps = {onClick, onKeyDown, onKeyUp, onMouseDown, onPointerDown, onPointerUp}; - return ( From fde31723a8689d0c0e22f4d59bc5b0da537f1c5c Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Fri, 31 Jul 2020 15:54:09 +0100 Subject: [PATCH 08/13] Linting fixes --- packages/@react-aria/select/src/useSelect.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 989940f7fcc..70f5dbdfcab 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -14,7 +14,7 @@ import {AriaButtonProps} from '@react-types/button'; import {AriaSelectProps} from '@react-types/select'; import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; import {HTMLAttributes, RefObject, useMemo} from 'react'; -import {KeyboardDelegate, PressEvents} from '@react-types/shared'; +import {KeyboardDelegate} from '@react-types/shared'; import {ListKeyboardDelegate, useTypeSelect} from '@react-aria/selection'; import {SelectState} from '@react-stately/select'; import {useCollator} from '@react-aria/i18n'; @@ -89,8 +89,10 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, ...labelProps, onClick: () => { const button = ref.current; - if(!button.disabled) ref.current.focus() - }, + if (!button.disabled) { + ref.current.focus(); + } + } }, triggerProps: mergeProps(domProps, { ...triggerProps, From a9d11ffa61747e7d3af78f09c145300b4ce67f5f Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Fri, 31 Jul 2020 16:33:05 +0100 Subject: [PATCH 09/13] Remove type casting --- packages/@react-aria/select/src/useSelect.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 70f5dbdfcab..8ecfb7f0ee4 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -88,8 +88,7 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, labelProps: { ...labelProps, onClick: () => { - const button = ref.current; - if (!button.disabled) { + if (!ref.current.disabled) { ref.current.focus(); } } From acd49fcf7cd4f1362be665fa1173a05c23e1ecf3 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Fri, 31 Jul 2020 16:44:23 +0100 Subject: [PATCH 10/13] Put typecast back --- packages/@react-aria/select/src/useSelect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 8ecfb7f0ee4..bcb55f58522 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -88,7 +88,7 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, labelProps: { ...labelProps, onClick: () => { - if (!ref.current.disabled) { + if (!(ref.current).disabled) { ref.current.focus(); } } From b47d2db092dd14d590904ad43305bff25f870147 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Sat, 1 Aug 2020 16:07:34 +0100 Subject: [PATCH 11/13] Use isDisabled from the props --- packages/@react-aria/select/src/useSelect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index bcb55f58522..4fe8535520d 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -88,7 +88,7 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, labelProps: { ...labelProps, onClick: () => { - if (!(ref.current).disabled) { + if (props.isDisabled) { ref.current.focus(); } } From b93927a3f637226393d98afee9613c4280cf1588 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Sat, 1 Aug 2020 16:09:15 +0100 Subject: [PATCH 12/13] Only focus when NOT disabled --- packages/@react-aria/select/src/useSelect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 4fe8535520d..8220d5696ad 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -88,7 +88,7 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, labelProps: { ...labelProps, onClick: () => { - if (props.isDisabled) { + if (!props.isDisabled) { ref.current.focus(); } } From 5d0e0a91099eccac0b2aa14f8f51ee059ffb8165 Mon Sep 17 00:00:00 2001 From: Gavin Henderson Date: Sat, 1 Aug 2020 17:53:17 +0100 Subject: [PATCH 13/13] Add test to make sure it focuses when you click the label --- .../@react-spectrum/picker/test/Picker.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/@react-spectrum/picker/test/Picker.test.js b/packages/@react-spectrum/picker/test/Picker.test.js index 1f29cb600a9..a937c215d8a 100644 --- a/packages/@react-spectrum/picker/test/Picker.test.js +++ b/packages/@react-spectrum/picker/test/Picker.test.js @@ -721,6 +721,24 @@ describe('Picker', function () { }); describe('labeling', function () { + it('focuses on the picker when you click the label', function () { + let {getAllByText, getByRole} = render( + + + One + Two + Three + + + ); + + let label = getAllByText('Test')[0]; + label.click(); + + let picker = getByRole('button'); + expect(document.activeElement).toBe(picker); + }); + it('supports labeling with a visible label', function () { let {getAllByText, getByText, getByRole} = render(