From 7ff475e41c325f0d8e9a310ab8a378b8f93341ac Mon Sep 17 00:00:00 2001
From: Jonathan Zempel <jzempel@gmail.com>
Date: Tue, 1 Aug 2023 16:22:26 -0400
Subject: [PATCH 1/4] fix(combobox): address multiselectable state issues with
 new `TagGroup` component

---
 packages/dropdowns.next/.size-snapshot.json   | 16 ++--
 .../src/elements/combobox/Combobox.tsx        | 88 +++++++++----------
 .../src/elements/combobox/TagGroup.tsx        | 41 +++++++++
 packages/dropdowns.next/src/types/index.ts    | 15 ++++
 packages/dropdowns.next/src/views/index.ts    |  1 +
 5 files changed, 109 insertions(+), 52 deletions(-)
 create mode 100644 packages/dropdowns.next/src/elements/combobox/TagGroup.tsx

diff --git a/packages/dropdowns.next/.size-snapshot.json b/packages/dropdowns.next/.size-snapshot.json
index 59c5e80d9e7..3440c79eaec 100644
--- a/packages/dropdowns.next/.size-snapshot.json
+++ b/packages/dropdowns.next/.size-snapshot.json
@@ -1,20 +1,20 @@
 {
   "index.cjs.js": {
-    "bundled": 55344,
-    "minified": 40274,
-    "gzipped": 9098
+    "bundled": 55791,
+    "minified": 40497,
+    "gzipped": 9252
   },
   "index.esm.js": {
-    "bundled": 50695,
-    "minified": 35869,
-    "gzipped": 8627,
+    "bundled": 51142,
+    "minified": 36092,
+    "gzipped": 8676,
     "treeshaked": {
       "rollup": {
-        "code": 28209,
+        "code": 28410,
         "import_statements": 1064
       },
       "webpack": {
-        "code": 31080
+        "code": 31283
       }
     }
   }
diff --git a/packages/dropdowns.next/src/elements/combobox/Combobox.tsx b/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
index 7c369a62c28..f1adca1a379 100644
--- a/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
+++ b/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
@@ -18,7 +18,7 @@ import React, {
 } from 'react';
 import PropTypes from 'prop-types';
 import { ThemeContext } from 'styled-components';
-import { IOption, IUseComboboxReturnValue, useCombobox } from '@zendeskgarden/container-combobox';
+import { IUseComboboxReturnValue, useCombobox } from '@zendeskgarden/container-combobox';
 import { DEFAULT_THEME, useText, useWindow } from '@zendeskgarden/react-theming';
 import { VALIDATION } from '@zendeskgarden/react-forms';
 import ChevronIcon from '@zendeskgarden/svg-icons/src/16/chevron-down-stroke.svg';
@@ -31,13 +31,13 @@ import {
   StyledInputIcon,
   StyledInput,
   StyledInputGroup,
+  StyledTagsButton,
   StyledTrigger,
   StyledValue
 } from '../../views';
-import { StyledTagsButton } from '../../views/combobox/StyledTagsButton';
 import { Listbox } from './Listbox';
-import { Tag } from './Tag';
-import { toOptions, toString } from './utils';
+import { TagGroup } from './TagGroup';
+import { toOptions } from './utils';
 
 const MAX_TAGS = 4;
 
@@ -82,6 +82,7 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
   ) => {
     const { hasHint, hasMessage, labelProps, setLabelProps } = useFieldContext();
     const [isLabelHovered, setIsLabelHovered] = useState(false);
+    const [isTagGroupExpanded, setIsTagGroupExpanded] = useState(false);
     const [optionTagProps, setOptionTagProps] = useState<Record<string, IOptionProps['tagProps']>>(
       {}
     );
@@ -99,7 +100,6 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
     const triggerRef = useRef<HTMLDivElement>(null);
     const inputRef = useRef<HTMLInputElement>(null);
     const listboxRef = useRef<HTMLUListElement>(null);
-    const tagsButtonRef = useRef<HTMLButtonElement>(null);
     /* istanbul ignore next */
     const theme = useContext(ThemeContext) || DEFAULT_THEME;
     const environment = useWindow(theme);
@@ -178,10 +178,18 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
       ...(getTriggerProps({
         onFocus: () => {
           hasFocus.current = true;
+
+          if (isMultiselectable) {
+            setIsTagGroupExpanded(true);
+          }
         },
         onBlur: event => {
           if (event.relatedTarget === null || !triggerRef.current?.contains(event.relatedTarget)) {
             hasFocus.current = false;
+
+            if (isMultiselectable) {
+              setIsTagGroupExpanded(false);
+            }
           }
         }
       }) as HTMLAttributes<HTMLDivElement>)
@@ -215,44 +223,6 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
       return () => labelProps && setLabelProps(undefined);
     }, [getLabelProps, labelProps, setLabelProps]);
 
-    const Tags = ({ selectedOptions }: { selectedOptions: IOption[] }) => {
-      const value = selectedOptions.length - maxTags;
-
-      return (
-        <>
-          {selectedOptions.map((option, index) => {
-            const key = toString(option);
-            const disabled = isDisabled || option.disabled;
-            const hidden = !hasFocus.current && index >= maxTags;
-
-            return (
-              <Tag
-                key={key}
-                hidden={hidden}
-                option={{ ...option, disabled }}
-                tooltipZIndex={listboxZIndex ? listboxZIndex + 1 : undefined}
-                {...optionTagProps[key]}
-              />
-            );
-          })}
-          {!hasFocus.current && selectedOptions.length > maxTags && (
-            <StyledTagsButton
-              disabled={isDisabled}
-              isCompact={isCompact}
-              onClick={() => isEditable && inputRef.current?.focus()}
-              tabIndex={-1}
-              type="button"
-              ref={tagsButtonRef}
-            >
-              {renderExpandTags
-                ? renderExpandTags(value)
-                : expandTags?.replace('{{value}}', value.toString())}
-            </StyledTagsButton>
-          )}
-        </>
-      );
-    };
-
     return (
       <ComboboxContext.Provider value={contextValue}>
         <StyledCombobox isCompact={isCompact} {...props} ref={ref}>
@@ -265,7 +235,37 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
               )}
               <StyledInputGroup>
                 {isMultiselectable && Array.isArray(selection) && (
-                  <Tags selectedOptions={selection} />
+                  <TagGroup
+                    isDisabled={isDisabled}
+                    isExpanded={isTagGroupExpanded}
+                    maxTags={maxTags}
+                    optionTagProps={optionTagProps}
+                    selection={selection}
+                  >
+                    {selection.length > maxTags && (
+                      <StyledTagsButton
+                        disabled={isDisabled}
+                        hidden={isTagGroupExpanded}
+                        isCompact={isCompact}
+                        onClick={event => {
+                          if (isEditable) {
+                            event.stopPropagation();
+                            inputRef.current?.focus();
+                          }
+                        }}
+                        tabIndex={-1}
+                        type="button"
+                      >
+                        {(() => {
+                          const value = selection.length - maxTags;
+
+                          return renderExpandTags
+                            ? renderExpandTags(value)
+                            : expandTags?.replace('{{value}}', value.toString());
+                        })()}
+                      </StyledTagsButton>
+                    )}
+                  </TagGroup>
                 )}
                 {!(isEditable && hasFocus.current) && (
                   <StyledValue
diff --git a/packages/dropdowns.next/src/elements/combobox/TagGroup.tsx b/packages/dropdowns.next/src/elements/combobox/TagGroup.tsx
new file mode 100644
index 00000000000..19a83ac81c0
--- /dev/null
+++ b/packages/dropdowns.next/src/elements/combobox/TagGroup.tsx
@@ -0,0 +1,41 @@
+/**
+ * Copyright Zendesk, Inc.
+ *
+ * Use of this source code is governed under the Apache License, Version 2.0
+ * found at http://www.apache.org/licenses/LICENSE-2.0.
+ */
+
+import React, { PropsWithChildren } from 'react';
+import { toString } from './utils';
+import { Tag } from './Tag';
+import { ITagGroupProps } from '../../types';
+
+export const TagGroup = ({
+  children,
+  isDisabled,
+  isExpanded,
+  listboxZIndex,
+  maxTags,
+  optionTagProps,
+  selection
+}: PropsWithChildren<ITagGroupProps>) => (
+  <>
+    {selection.map((option, index) => {
+      const key = toString(option);
+      const disabled = isDisabled || option.disabled;
+
+      return (
+        <Tag
+          key={key}
+          hidden={!isExpanded && index >= maxTags}
+          option={{ ...option, disabled }}
+          tooltipZIndex={listboxZIndex ? listboxZIndex + 1 : undefined}
+          {...optionTagProps[key]}
+        />
+      );
+    })}
+    {children}
+  </>
+);
+
+TagGroup.displayName = 'TagGroup';
diff --git a/packages/dropdowns.next/src/types/index.ts b/packages/dropdowns.next/src/types/index.ts
index 642a14c48ac..d7612cd31ec 100644
--- a/packages/dropdowns.next/src/types/index.ts
+++ b/packages/dropdowns.next/src/types/index.ts
@@ -237,3 +237,18 @@ export interface ITagProps extends Omit<IBaseTagProps, 'isRound' | 'size'> {
   /** @ignore Sets the `z-index` of the tooltip */
   tooltipZIndex?: number;
 }
+
+export interface ITagGroupProps {
+  /** Indicates that the tag group is not interactive */
+  isDisabled?: boolean;
+  /** Determines tag group expansion */
+  isExpanded: boolean;
+  /** Indicates the `z-index` of the listbox */
+  listboxZIndex?: number;
+  /** Determines the maximum number of tags displayed when the tag group is collapsed */
+  maxTags: number;
+  /** Provides tag props for the associated option */
+  optionTagProps: Record<string, IOptionProps['tagProps']>;
+  /** Provides the current selection */
+  selection: IOption[];
+}
diff --git a/packages/dropdowns.next/src/views/index.ts b/packages/dropdowns.next/src/views/index.ts
index f3bfff2e2ed..5474e300998 100644
--- a/packages/dropdowns.next/src/views/index.ts
+++ b/packages/dropdowns.next/src/views/index.ts
@@ -24,6 +24,7 @@ export * from './combobox/StyledOptionIcon';
 export * from './combobox/StyledOptionMeta';
 export * from './combobox/StyledOptionTypeIcon';
 export * from './combobox/StyledTag';
+export * from './combobox/StyledTagsButton';
 export * from './combobox/StyledTrigger';
 export * from './combobox/StyledValue';
 export * from './menu/StyledFloatingMenu';

From e5a633c8a5ce1ab0b15efb9c90103d5c624b37ca Mon Sep 17 00:00:00 2001
From: Jonathan Zempel <jzempel@gmail.com>
Date: Wed, 2 Aug 2023 08:49:51 -0400
Subject: [PATCH 2/4] Hack aria-labelledby for JAWS + NVDA

---
 packages/dropdowns.next/.size-snapshot.json      | 16 ++++++++--------
 .../src/elements/combobox/Combobox.spec.tsx      |  2 ++
 .../src/elements/combobox/Combobox.tsx           |  7 ++++++-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/packages/dropdowns.next/.size-snapshot.json b/packages/dropdowns.next/.size-snapshot.json
index 3440c79eaec..193efdd1ebb 100644
--- a/packages/dropdowns.next/.size-snapshot.json
+++ b/packages/dropdowns.next/.size-snapshot.json
@@ -1,20 +1,20 @@
 {
   "index.cjs.js": {
-    "bundled": 55791,
-    "minified": 40497,
-    "gzipped": 9252
+    "bundled": 55834,
+    "minified": 40518,
+    "gzipped": 9261
   },
   "index.esm.js": {
-    "bundled": 51142,
-    "minified": 36092,
-    "gzipped": 8676,
+    "bundled": 51185,
+    "minified": 36113,
+    "gzipped": 8685,
     "treeshaked": {
       "rollup": {
-        "code": 28410,
+        "code": 28431,
         "import_statements": 1064
       },
       "webpack": {
-        "code": 31283
+        "code": 31304
       }
     }
   }
diff --git a/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx b/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
index 843ee001541..e70cf8a3598 100644
--- a/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
+++ b/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
@@ -220,8 +220,10 @@ describe('Combobox', () => {
 
   it('renders non-editable as expected', () => {
     const { getByTestId } = render(<TestCombobox isEditable={false} />);
+    const combobox = getByTestId('combobox');
     const input = getByTestId('input');
 
+    expect(combobox).toHaveAttribute('tabIndex', '-1');
     expect(input).toHaveAttribute('readonly');
     expect(input).toHaveAttribute('hidden');
   });
diff --git a/packages/dropdowns.next/src/elements/combobox/Combobox.tsx b/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
index f1adca1a379..c5982e1eac0 100644
--- a/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
+++ b/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
@@ -225,7 +225,12 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
 
     return (
       <ComboboxContext.Provider value={contextValue}>
-        <StyledCombobox isCompact={isCompact} {...props} ref={ref}>
+        <StyledCombobox
+          isCompact={isCompact}
+          tabIndex={isEditable ? undefined : -1} // HACK: otherwise JAWS + NVDA can't read the label
+          {...props}
+          ref={ref}
+        >
           <StyledTrigger {...triggerProps}>
             <StyledContainer>
               {startIcon && (

From 9a22533f68abbde38ee46a88da20c9ef5f85da9c Mon Sep 17 00:00:00 2001
From: Jonathan Zempel <jzempel@gmail.com>
Date: Wed, 2 Aug 2023 10:14:54 -0400
Subject: [PATCH 3/4] Fill out tag group expansion unit testing

---
 .../src/elements/combobox/Combobox.spec.tsx   | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx b/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
index e70cf8a3598..37d75f90535 100644
--- a/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
+++ b/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
@@ -351,6 +351,41 @@ describe('Combobox', () => {
     expect(button).toHaveTextContent('test-1');
   });
 
+  it('handles tag group expansion as expected', async () => {
+    const tagProps = { 'data-test-id': 'tag' } as HTMLAttributes<HTMLDivElement>;
+    const { getByTestId } = render(
+      <TestCombobox isMultiselectable maxTags={1}>
+        <Option isSelected value="one" />
+        <Option isSelected tagProps={tagProps} value="two" />
+      </TestCombobox>
+    );
+    const combobox = getByTestId('combobox');
+    const trigger = combobox.firstChild as HTMLElement;
+    const input = getByTestId('input');
+    const tag = getByTestId('tag');
+    const button = tag.nextSibling as HTMLElement;
+
+    expect(tag).toHaveAttribute('hidden');
+    expect(button).not.toHaveAttribute('hidden');
+
+    await user.click(button);
+
+    expect(tag).not.toHaveAttribute('hidden');
+    expect(button).toHaveAttribute('hidden');
+    expect(input).toHaveFocus();
+
+    await user.keyboard('{Tab}');
+
+    expect(tag).toHaveAttribute('hidden');
+    expect(button).not.toHaveAttribute('hidden');
+
+    await user.click(trigger);
+
+    expect(tag).not.toHaveAttribute('hidden');
+    expect(button).toHaveAttribute('hidden');
+    expect(input).toHaveFocus();
+  });
+
   it('handles `renderValue` as expected', () => {
     const { getByTestId } = render(
       <TestCombobox renderValue={({ selection }) => `test-${(selection as ISelectedOption).value}`}>

From f2770562419b2ca2218e77e951871ffad07aed30 Mon Sep 17 00:00:00 2001
From: Jonathan Zempel <jzempel@gmail.com>
Date: Thu, 3 Aug 2023 08:19:49 -0400
Subject: [PATCH 4/4] Prevent double screenreader readout of value

---
 packages/dropdowns.next/.size-snapshot.json      | 16 ++++++++--------
 .../src/elements/combobox/Combobox.spec.tsx      |  2 ++
 .../src/elements/combobox/Combobox.tsx           |  3 ++-
 .../src/views/combobox/StyledInput.ts            |  7 ++++++-
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/packages/dropdowns.next/.size-snapshot.json b/packages/dropdowns.next/.size-snapshot.json
index 193efdd1ebb..b71158ed689 100644
--- a/packages/dropdowns.next/.size-snapshot.json
+++ b/packages/dropdowns.next/.size-snapshot.json
@@ -1,20 +1,20 @@
 {
   "index.cjs.js": {
-    "bundled": 55834,
-    "minified": 40518,
-    "gzipped": 9261
+    "bundled": 55890,
+    "minified": 40577,
+    "gzipped": 9273
   },
   "index.esm.js": {
-    "bundled": 51185,
-    "minified": 36113,
-    "gzipped": 8685,
+    "bundled": 51241,
+    "minified": 36172,
+    "gzipped": 8697,
     "treeshaked": {
       "rollup": {
-        "code": 28431,
+        "code": 28490,
         "import_statements": 1064
       },
       "webpack": {
-        "code": 31304
+        "code": 31363
       }
     }
   }
diff --git a/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx b/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
index 37d75f90535..85049f567f2 100644
--- a/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
+++ b/packages/dropdowns.next/src/elements/combobox/Combobox.spec.tsx
@@ -226,6 +226,8 @@ describe('Combobox', () => {
     expect(combobox).toHaveAttribute('tabIndex', '-1');
     expect(input).toHaveAttribute('readonly');
     expect(input).toHaveAttribute('hidden');
+    expect(input).toHaveAttribute('aria-hidden', 'true');
+    expect(input).toHaveStyleRule('display', 'none', { modifier: '&[aria-hidden="true"]' });
   });
 
   it('renders `isMultiselectable` as expected', () => {
diff --git a/packages/dropdowns.next/src/elements/combobox/Combobox.tsx b/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
index c5982e1eac0..63e57a80973 100644
--- a/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
+++ b/packages/dropdowns.next/src/elements/combobox/Combobox.tsx
@@ -199,6 +199,7 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
       hidden: !(isEditable && hasFocus.current),
       isBare,
       isCompact,
+      isEditable,
       isMultiselectable,
       placeholder,
       ...(getInputProps({
@@ -227,7 +228,7 @@ export const Combobox = forwardRef<HTMLDivElement, IComboboxProps>(
       <ComboboxContext.Provider value={contextValue}>
         <StyledCombobox
           isCompact={isCompact}
-          tabIndex={isEditable ? undefined : -1} // HACK: otherwise JAWS + NVDA can't read the label
+          tabIndex={-1} // HACK: otherwise screenreaders can't read the label
           {...props}
           ref={ref}
         >
diff --git a/packages/dropdowns.next/src/views/combobox/StyledInput.ts b/packages/dropdowns.next/src/views/combobox/StyledInput.ts
index 370e38dc6ed..1f695c8c2b2 100644
--- a/packages/dropdowns.next/src/views/combobox/StyledInput.ts
+++ b/packages/dropdowns.next/src/views/combobox/StyledInput.ts
@@ -19,6 +19,7 @@ const COMPONENT_ID = 'dropdowns.combobox.input';
 interface IStyledInputProps extends ThemeProps<DefaultTheme> {
   isBare?: boolean;
   isCompact?: boolean;
+  isEditable?: boolean;
   isMultiselectable?: boolean;
 }
 
@@ -84,7 +85,11 @@ export const StyledInput = styled.input.attrs({
 
   &[hidden] {
     display: revert;
-    ${hideVisually()}
+    ${props => props.isEditable && hideVisually()}
+  }
+
+  &[aria-hidden='true'] {
+    display: none;
   }
 
   ${props => retrieveComponentStyles(COMPONENT_ID, props)};