RAC: Fix Group render props and focus-visible behavior with text input children (#5359)

Fixes:
- Group now works with render props, no longer errors with "Warning: Functions are not valid as a React child."
- Group's focus ring will render correctly with text input children.

Author: reidbarber

packages/@react-aria/interactions/src/useFocusVisible.ts

@@ -196,11 +196,27 @@ export function useInteractionModality(): Modality | null {
   return useIsSSR() ? null : modality;
 }
 
+const nonTextInputTypes = new Set([
+  'checkbox',
+  'radio',
+  'range',
+  'color',
+  'file',
+  'image',
+  'button',
+  'submit',
+  'reset'
+]);
+
 /**
  * If this is attached to text input component, return if the event is a focus event (Tab/Escape keys pressed) so that
  * focus visible style can be properly set.
  */
 function isKeyboardFocusEvent(isTextInput: boolean, modality: Modality, e: HandlerEvent) {
+  isTextInput = isTextInput || 
+    (e?.target instanceof HTMLInputElement && !nonTextInputTypes.has(e?.target?.type)) ||
+    e?.target instanceof HTMLTextAreaElement ||
+    (e?.target instanceof HTMLElement && e?.target.isContentEditable);
   return !(isTextInput && modality === 'keyboard' && e instanceof KeyboardEvent && !FOCUS_VISIBLE_INPUT_KEYS[e.key]);
 }
 

packages/react-aria-components/src/Group.tsx

@@ -10,9 +10,9 @@
  * governing permissions and limitations under the License.
  */
 
-import {AriaLabelingProps, HoverEvents} from '@react-types/shared';
-import {ContextValue, StyleRenderProps, useContextProps, useRenderProps} from './utils';
-import {mergeProps, useFocusRing, useHover} from 'react-aria';
+import {AriaLabelingProps, DOMProps} from '@react-types/shared';
+import {ContextValue, forwardRefType, RenderProps, SlotProps, useContextProps, useRenderProps} from './utils';
+import {HoverProps, mergeProps, useFocusRing, useHover} from 'react-aria';
 import React, {createContext, ForwardedRef, forwardRef, HTMLAttributes} from 'react';
 
 export interface GroupRenderProps {
@@ -43,7 +43,7 @@ export interface GroupRenderProps {
   isInvalid: boolean
 }
 
-export interface GroupProps extends AriaLabelingProps, Omit<HTMLAttributes<HTMLElement>, 'className' | 'style' | 'role'>, HoverEvents, StyleRenderProps<GroupRenderProps> {
+export interface GroupProps extends AriaLabelingProps, Omit<HTMLAttributes<HTMLElement>, 'children' | 'className' | 'style' | 'role' | 'slot'>, DOMProps, HoverProps, RenderProps<GroupRenderProps>, SlotProps {
   /** Whether the group is disabled. */
   isDisabled?: boolean,
   /** Whether the group is invalid. */
@@ -62,13 +62,13 @@ export const GroupContext = createContext<ContextValue<GroupProps, HTMLDivElemen
 
 function Group(props: GroupProps, ref: ForwardedRef<HTMLDivElement>) {
   [props, ref] = useContextProps(props, ref, GroupContext);
+  let {isDisabled, isInvalid, onHoverStart, onHoverChange, onHoverEnd, ...otherProps} = props;
 
-  let {hoverProps, isHovered} = useHover(props);
+  let {hoverProps, isHovered} = useHover({onHoverStart, onHoverChange, onHoverEnd, isDisabled});
   let {isFocused, isFocusVisible, focusProps} = useFocusRing({
     within: true
   });
 
-  let {isDisabled, isInvalid, ...otherProps} = props;
   isDisabled ??= !!props['aria-disabled'] && props['aria-disabled'] !== 'false';
   isInvalid ??= !!props['aria-invalid'] && props['aria-invalid'] !== 'false';
   let renderProps = useRenderProps({
@@ -83,18 +83,19 @@ function Group(props: GroupProps, ref: ForwardedRef<HTMLDivElement>) {
       {...renderProps}
       ref={ref}
       role={props.role ?? 'group'}
+      slot={props.slot ?? undefined}
       data-focus-within={isFocused || undefined}
       data-hovered={isHovered || undefined}
       data-focus-visible={isFocusVisible || undefined}
       data-disabled={isDisabled || undefined}
       data-invalid={isInvalid || undefined}>
-      {props.children}
+      {renderProps.children}
     </div>
   );
 }
 
 /**
  * A group represents a set of related UI controls, and supports interactive states for styling.
  */
-const _Group = forwardRef(Group);
+const _Group = /*#__PURE__*/ (forwardRef as forwardRefType)(Group);
 export {_Group as Group};

packages/react-aria-components/test/Group.test.tsx

@@ -0,0 +1,139 @@
+/*
+ * Copyright 2023 Adobe. All rights reserved.
+ * This file is licensed to you under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License. You may obtain a copy
+ * of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under
+ * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+ * OF ANY KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+import {Group, GroupContext} from '..';
+import {pointerMap, render} from '@react-spectrum/test-utils';
+import React from 'react';
+import userEvent from '@testing-library/user-event';
+
+describe('Group', () => {
+  let user;
+  beforeAll(() => {
+    user = userEvent.setup({delay: null, pointerMap});
+  });
+
+  it('should render a group with default class', () => {
+    let {getByRole} = render(<Group>Test</Group>);
+    let group = getByRole('group');
+    expect(group).toHaveAttribute('class', 'react-aria-Group');
+  });
+
+  it('should render a group with custom class', () => {
+    let {getByRole} = render(<Group className="test">Test</Group>);
+    let group = getByRole('group');
+    expect(group).toHaveAttribute('class', 'test');
+  });
+
+  it('should support DOM props', () => {
+    let {getByRole} = render(<Group data-foo="bar">Test</Group>);
+    let group = getByRole('group');
+    expect(group).toHaveAttribute('data-foo', 'bar');
+  });
+
+  it('should support slot', () => {
+    let {getByRole} = render(
+      <GroupContext.Provider value={{slots: {test: {'aria-label': 'test'}}}}>
+        <Group slot="test">Test</Group>
+      </GroupContext.Provider>
+    );
+
+    let group = getByRole('group');
+    expect(group).toHaveAttribute('slot', 'test');
+    expect(group).toHaveAttribute('aria-label', 'test');
+  });
+
+  it('should support hover', async () => {
+    let hoverStartSpy = jest.fn();
+    let hoverChangeSpy = jest.fn();
+    let hoverEndSpy = jest.fn();
+    let {getByRole} = render(<Group
+      className={({isHovered}) => isHovered ? 'hover' : ''}
+      onHoverStart={hoverStartSpy}
+      onHoverChange={hoverChangeSpy}
+      onHoverEnd={hoverEndSpy}>Test</Group>);
+    let group = getByRole('group');
+
+    expect(group).not.toHaveAttribute('data-hovered');
+    expect(group).not.toHaveClass('hover');
+
+    await user.hover(group);
+    expect(group).toHaveAttribute('data-hovered', 'true');
+    expect(group).toHaveClass('hover');
+    expect(hoverStartSpy).toHaveBeenCalledTimes(1);
+    expect(hoverChangeSpy).toHaveBeenCalledTimes(1);
+
+    await user.unhover(group);
+    expect(group).not.toHaveAttribute('data-hovered');
+    expect(group).not.toHaveClass('hover');
+    expect(hoverEndSpy).toHaveBeenCalledTimes(1);
+    expect(hoverChangeSpy).toHaveBeenCalledTimes(2);
+  });
+
+  it('should support focus ring', async () => {
+    let {getByRole} = render(<Group className={({isFocusVisible}) => isFocusVisible ? 'focus' : ''}>
+      <input type="text" />
+    </Group>);
+    let group = getByRole('group');
+    let input = getByRole('textbox');
+
+    expect(group).not.toHaveAttribute('data-focus-visible');
+    expect(group).not.toHaveClass('focus');
+
+    await user.tab();
+    expect(document.activeElement).toBe(input);
+    expect(group).toHaveAttribute('data-focus-visible', 'true');
+    expect(group).toHaveClass('focus');
+
+    await user.tab();
+    expect(group).not.toHaveAttribute('data-focus-visible');
+    expect(group).not.toHaveClass('focus');
+  });
+
+  it('should not show focus ring when typing in pointer modality', async () => {
+    let {getByRole} = render(<Group className={({isFocusVisible}) => isFocusVisible ? 'focus' : ''}>
+      <input type="text" />
+    </Group>);
+    let group = getByRole('group');
+    let input = getByRole('textbox');
+
+    expect(group).not.toHaveAttribute('data-focus-visible');
+    expect(group).not.toHaveClass('focus');
+
+    await user.click(input);
+    await user.keyboard('a');
+    expect(document.activeElement).toBe(input);
+    expect(input).toHaveValue('a');
+
+    expect(group).not.toHaveAttribute('data-focus-visible');
+    expect(group).not.toHaveClass('focus');
+  });
+
+  it('should support disabled state', () => {
+    let {getByRole} = render(<Group isDisabled className={({isDisabled}) => isDisabled ? 'disabled' : ''}>Test</Group>);
+    let group = getByRole('group');
+
+    expect(group).toHaveAttribute('data-disabled', 'true');
+    expect(group).toHaveClass('disabled');
+  });
+
+  it('should support render props', async () => {
+    let {getByRole} = render(<Group>{({isHovered}) => isHovered ? 'Hovered' : 'Group'}</Group>);
+    let group = getByRole('group');
+
+    expect(group).toHaveTextContent('Group');
+
+    await user.hover(group);
+    expect(group).toHaveTextContent('Hovered');
+
+    await user.unhover(group);
+    expect(group).toHaveTextContent('Group');
+  });
+});