chore: Enhance OUIA IDs for ConditionalFilter (#1972)

* chore: Enhance OUIA IDs for ConditionalFilter * chore: Cover more bulk select components with OUIA ID * chore: Add more OUIA IDs to primary toolbar actions * chore: Add OUIA ID for bulk select dropdown * chore: Update remaining snapshots * chore: Update cypress tests
RedHatInsights · Feb 22, 2024 · 56b406c · 56b406c
1 parent eb63322
commit 56b406c
Show file tree

Hide file tree

Showing 10 changed files with 76 additions and 47 deletions.
diff --git a/packages/components/src/BulkSelect/BulkSelect.tsx b/packages/components/src/BulkSelect/BulkSelect.tsx
@@ -1,5 +1,6 @@
 import React, { Fragment, useRef, useState } from 'react';
 import classnames from 'classnames';
+// eslint-disable-next-line rulesdir/forbid-pf-relative-imports
 import {
   Checkbox,
   Dropdown,
@@ -10,7 +11,6 @@ import {
   MenuToggleCheckbox,
   MenuToggleCheckboxProps,
   MenuToggleProps,
-  getDefaultOUIAId,
 } from '@patternfly/react-core';
 
 import './bulk-select.scss';
@@ -35,8 +35,10 @@ export interface BulkSelectProps {
   onSelect?: MenuToggleCheckboxProps['onChange'];
   toggleProps?: MenuToggleProps;
   isDisabled?: boolean;
-  ouiaId?: string;
   ouiaSafe?: boolean;
+  dropdownOuiaId?: string;
+  checkboxOuiaId?: string;
+  listOuiaId?: string;
 }
 
 const BulkSelect: React.FunctionComponent<BulkSelectProps> = ({
@@ -48,36 +50,34 @@ const BulkSelect: React.FunctionComponent<BulkSelectProps> = ({
   toggleProps,
   count,
   className,
-  ouiaId,
   ouiaSafe = true,
   ...props
 }) => {
   const [isOpen, setIsOpen] = useState(false);
   const { current: hasError } = useRef(false);
-  const { current: ouiaStateId } = useRef(getDefaultOUIAId('RHI/BulkSelect'));
 
   const onToggle = (isOpen: boolean) => setIsOpen(isOpen);
 
-  const ouiaFinalId = ouiaId !== undefined ? ouiaId : ouiaStateId;
+  const { dropdownOuiaId, checkboxOuiaId, listOuiaId } = props;
 
   return (
     <Fragment>
       {items && items.length > 0 ? (
         <Dropdown
           onSelect={() => onToggle(false)}
           onOpenChange={(isOpen: boolean) => setIsOpen(isOpen)}
+          data-ouia-component-id={dropdownOuiaId ?? 'BulkSelect'}
           {...props}
           className={classnames(className, 'ins-c-bulk-select')}
-          ouiaId={ouiaFinalId}
           ouiaSafe={ouiaSafe}
           toggle={(toggleRef) => (
             <MenuToggle
-              aria-label={ouiaFinalId}
               {...toggleProps}
               isDisabled={isDisabled}
               isExpanded={isOpen}
               ref={toggleRef}
               onClick={() => setIsOpen((prev) => !prev)}
+              data-ouia-component-id={dropdownOuiaId ?? 'BulkSelect'}
             >
               <Fragment key="split-checkbox">
                 {hasError ? (
@@ -86,15 +86,15 @@ const BulkSelect: React.FunctionComponent<BulkSelectProps> = ({
                     aria-label="Select all"
                     onChange={onSelect}
                     isChecked={checked}
-                    ouiaId={ouiaFinalId}
+                    ouiaId={checkboxOuiaId ?? 'BulkSelectCheckbox'}
                   />
                 ) : (
                   <MenuToggleCheckbox
                     id={id ? `${id}-toggle-checkbox` : 'toggle-checkbox'}
                     aria-label="Select all"
                     onChange={onSelect}
                     isChecked={checked}
-                    ouiaId={ouiaFinalId}
+                    ouiaId={checkboxOuiaId ?? 'BulkSelectCheckbox'}
                   >
                     {count ? `${count} selected` : ''}
                   </MenuToggleCheckbox>
@@ -104,7 +104,7 @@ const BulkSelect: React.FunctionComponent<BulkSelectProps> = ({
           )}
           isOpen={isOpen}
         >
-          <DropdownList>
+          <DropdownList data-ouia-component-id={listOuiaId ?? 'BulkSelectList'}>
             {count !== undefined && count > 0 && (
               <DropdownItem
                 key="count"
@@ -120,7 +120,7 @@ const BulkSelect: React.FunctionComponent<BulkSelectProps> = ({
               <DropdownItem
                 component="button"
                 key={oneItem.key || key}
-                ouiaId={`${ouiaFinalId}-${oneItem.key || key}`}
+                ouiaId={`${listOuiaId ?? 'BulkSelectList'}-${oneItem.key || key}`}
                 onClick={(event) => oneItem.onClick && oneItem.onClick(event, oneItem, key)}
                 {...oneItem?.props}
               >
@@ -137,6 +137,7 @@ const BulkSelect: React.FunctionComponent<BulkSelectProps> = ({
           id={`${id}-checkbox`}
           isChecked={checked}
           onChange={(e, checked) => onSelect?.(checked, e)}
+          ouiaId={dropdownOuiaId ?? 'BulkSelect'}
         />
       )}
     </Fragment>

diff --git a/packages/components/src/BulkSelect/__snapshots__/BulkSelect.test.js.snap b/packages/components/src/BulkSelect/__snapshots__/BulkSelect.test.js.snap
@@ -9,7 +9,7 @@ exports[`BulkSelect should render correctly - no data 1`] = `
       aria-invalid="false"
       aria-label="Select all"
       class="pf-v5-c-check__input"
-      data-ouia-component-id="OUIA-Generated-Checkbox-1"
+      data-ouia-component-id="BulkSelect"
       data-ouia-component-type="PF5/Checkbox"
       data-ouia-safe="true"
       id="undefined-checkbox"
@@ -23,8 +23,8 @@ exports[`BulkSelect should render correctly - null checked 1`] = `
 <div>
   <button
     aria-expanded="false"
-    aria-label="OUIA-Generated-RHI/BulkSelect-4"
     class="pf-v5-c-menu-toggle"
+    data-ouia-component-id="BulkSelect"
     type="button"
   >
     <span
@@ -38,7 +38,7 @@ exports[`BulkSelect should render correctly - null checked 1`] = `
           aria-invalid="false"
           aria-label="Select all"
           class="pf-v5-c-check__input"
-          data-ouia-component-id="OUIA-Generated-RHI/BulkSelect-4"
+          data-ouia-component-id="BulkSelectCheckbox"
           data-ouia-component-type="PF5/MenuToggleCheckbox"
           data-ouia-safe="true"
           name="toggle-checkbox"
@@ -75,8 +75,8 @@ exports[`BulkSelect should render correctly 1`] = `
 <div>
   <button
     aria-expanded="false"
-    aria-label="OUIA-Generated-RHI/BulkSelect-2"
     class="pf-v5-c-menu-toggle"
+    data-ouia-component-id="BulkSelect"
     type="button"
   >
     <span
@@ -90,7 +90,7 @@ exports[`BulkSelect should render correctly 1`] = `
           aria-invalid="false"
           aria-label="Select all"
           class="pf-v5-c-check__input"
-          data-ouia-component-id="OUIA-Generated-RHI/BulkSelect-2"
+          data-ouia-component-id="BulkSelectCheckbox"
           data-ouia-component-type="PF5/MenuToggleCheckbox"
           data-ouia-safe="true"
           name="some-id-toggle-checkbox"
@@ -134,8 +134,8 @@ exports[`BulkSelect should render correctly 2`] = `
 <div>
   <button
     aria-expanded="false"
-    aria-label="OUIA-Generated-RHI/BulkSelect-3"
     class="pf-v5-c-menu-toggle"
+    data-ouia-component-id="BulkSelect"
     type="button"
   >
     <span
@@ -149,7 +149,7 @@ exports[`BulkSelect should render correctly 2`] = `
           aria-invalid="false"
           aria-label="Select all"
           class="pf-v5-c-check__input"
-          data-ouia-component-id="OUIA-Generated-RHI/BulkSelect-3"
+          data-ouia-component-id="BulkSelectCheckbox"
           data-ouia-component-type="PF5/MenuToggleCheckbox"
           data-ouia-safe="true"
           name="toggle-checkbox"
@@ -186,8 +186,8 @@ exports[`BulkSelect should render custom props 1`] = `
 <div>
   <button
     aria-expanded="false"
-    aria-label="OUIA-Generated-RHI/BulkSelect-5"
     class="pf-v5-c-menu-toggle"
+    data-ouia-component-id="BulkSelect"
     type="button"
   >
     <span
@@ -201,7 +201,7 @@ exports[`BulkSelect should render custom props 1`] = `
           aria-invalid="false"
           aria-label="Select all"
           class="pf-v5-c-check__input"
-          data-ouia-component-id="OUIA-Generated-RHI/BulkSelect-5"
+          data-ouia-component-id="BulkSelectCheckbox"
           data-ouia-component-type="PF5/MenuToggleCheckbox"
           data-ouia-safe="true"
           name="toggle-checkbox"

diff --git a/packages/components/src/ConditionalFilter/ConditionalFilter.test.js b/packages/components/src/ConditionalFilter/ConditionalFilter.test.js
@@ -131,16 +131,16 @@ describe('ConditionalFilter', () => {
     it('should open dropdown', async () => {
       render(<ConditionalFilter {...initialProps} items={config} />);
       await act(async () => {
-        await userEvent.click(screen.getByRole('button', { name: 'Conditional filter' }));
+        await userEvent.click(screen.getByRole('button', { name: 'Conditional filter toggle' }));
       });
-      expect(screen.getByRole('menu', { name: 'Conditional filter' })).toBeDefined();
+      expect(screen.getByLabelText('Conditional filters list')).toBeDefined();
     });
 
     it('should call NOT call onChange when clicked on dropdown', async () => {
       const onChange = jest.fn();
       render(<ConditionalFilter {...initialProps} items={config} />);
       act(() => {
-        userEvent.click(screen.getByRole('button', { name: 'Conditional filter' }));
+        userEvent.click(screen.getByRole('button', { name: 'Conditional filter toggle' }));
       });
       await act(async () => {
         userEvent.click(screen.getByRole('menuitem', { name: 'Simple text 1' }));
@@ -152,7 +152,7 @@ describe('ConditionalFilter', () => {
       const onChange = jest.fn();
       render(<ConditionalFilter {...initialProps} items={config} onChange={onChange} />);
       act(() => {
-        userEvent.click(screen.getByRole('button', { name: 'Conditional filter' }));
+        userEvent.click(screen.getByRole('button', { name: 'Conditional filter toggle' }));
       });
       await act(async () => {
         await userEvent.click(screen.getByRole('menuitem', { name: 'Simple text 1' }));
@@ -163,7 +163,7 @@ describe('ConditionalFilter', () => {
     it('should update state on select', async () => {
       render(<ConditionalFilter {...initialProps} items={config} />);
       act(() => {
-        userEvent.click(screen.getByRole('button', { name: 'Conditional filter' }));
+        userEvent.click(screen.getByRole('button', { name: 'Conditional filter toggle' }));
       });
 
       await act(async () => {

diff --git a/packages/components/src/ConditionalFilter/ConditionalFilter.tsx b/packages/components/src/ConditionalFilter/ConditionalFilter.tsx
@@ -193,18 +193,19 @@ const ConditionalFilter: React.FunctionComponent<ConditionalFilterProps> = ({
                     ref={innerRef}
                     onSelect={() => setIsOpen(false)}
                     isOpen={isOpen}
-                    ouiaId="ConditionalFilter"
+                    ouiaId="ConditionalFilterList"
                     onOpenChange={(isOpen: boolean) => setIsOpen(isOpen)}
                     toggle={(toggleRef) => (
                       <MenuToggle
                         isDisabled={isDisabled}
                         className={
                           hideLabel ? 'ins-c-conditional-filter__group ins-c-conditional-filter__no-label' : 'ins-c-conditional-filter__group'
                         }
-                        aria-label="Conditional filter"
+                        aria-label="Conditional filter toggle"
                         ref={toggleRef}
                         onClick={() => setIsOpen((prev) => !prev)}
                         isExpanded={isOpen}
+                        data-ouia-component-id="ConditionalFilterToggle"
                       >
                         <Icon size="md">
                           <FilterIcon />
@@ -215,7 +216,7 @@ const ConditionalFilter: React.FunctionComponent<ConditionalFilterProps> = ({
                       </MenuToggle>
                     )}
                   >
-                    <DropdownList aria-label="Conditional filter">
+                    <DropdownList aria-label="Conditional filters list">
                       {items.map((item, key) => (
                         <DropdownItem
                           key={item.id ? `${item.id}-dropdown` : key}
@@ -230,7 +231,11 @@ const ConditionalFilter: React.FunctionComponent<ConditionalFilterProps> = ({
                   </Dropdown>
                 </SplitItem>
               )}
-              {ActiveComponent && <SplitItem isFilled>{getActiveComponent(activeItem)}</SplitItem>}
+              {ActiveComponent && (
+                <SplitItem isFilled data-ouia-component-id="ConditionalFilter">
+                  {getActiveComponent(activeItem)}
+                </SplitItem>
+              )}
             </Split>
           )}
         </Fragment>