Prevent menuAttributes from overriding attributes set automatically b…

…y the component

README.md

@@ -209,6 +209,13 @@ Type: `Object`
 
 Sets html attributes and their values on the `menu`. Useful for adding `aria-labelledby` and setting to the value of the `id` attribute on your existing label, to provide context to an assistive technology user.
 
+> [!NOTE]
+>
+> This option will not override the attributes computed by the component
+> - `id` and `role` are dropped to limit risks to accessibility
+> - `className` and `class` are appended to the `class` attribute of the menu so it keeps the classes computed by the component
+
+
 #### `menuClasses` (default: `null`)
 
 Type: `String`

src/autocomplete.js

@@ -492,12 +492,20 @@ export default class Autocomplete extends Component {
     const menuIsVisible = menuOpen || showNoOptionsFound
     const menuModifierVisibility = `${menuClassName}--${(menuIsVisible) ? 'visible' : 'hidden'}`
 
+    const { classList, ...safeMenuAttributes } = sanitiseMenuAttributes(menuAttributes || {})
+
     const menuClassList = [
       menuClassName,
       menuModifierDisplayMenu,
-      menuModifierVisibility
+      menuModifierVisibility,
+      ...classList
     ]
 
+    if (sanitiseMenuAttributes.class) {
+      menuClassList.push(safeMenuAttributes.class)
+      delete safeMenuAttributes.class
+    }
+
     if (menuClasses) {
       menuClassList.push(menuClasses)
     }
@@ -549,7 +557,7 @@ export default class Autocomplete extends Component {
           onMouseLeave={(event) => this.handleListMouseLeave(event)}
           id={`${id}__listbox`}
           role='listbox'
-          {...menuAttributes}
+          {...safeMenuAttributes}
         >
           {options.map((option, index) => {
             const showFocused = focused === -1 ? selected === index : focused === index
@@ -592,3 +600,21 @@ export default class Autocomplete extends Component {
     )
   }
 }
+
+/**
+ * Sanitise the attributes being assigned in bulk
+ * @param {Object} unsafeMenuAttributes
+ */
+function sanitiseMenuAttributes (unsafeMenuAttributes) {
+  // We need to extract both the `className` (for both React and Preact)
+  // and `class` properties (for Preact) that would override `className` in JSX
+  // then store them in a way we can re-use them
+  const { className, class: classProperty, ...result } = unsafeMenuAttributes
+  result.classList = [className, classProperty]
+
+  // As well as the attributes we use
+  delete result.id
+  delete result.role
+
+  return result
+}

test/functional/index.js

@@ -110,15 +110,42 @@ describe('Autocomplete', () => {
         })
       })
 
-      it('renders with an aria-labelledby attribute on the menu', () => {
-        render(<Autocomplete menuAttributes={{ 'aria-labelledby': 'autocomplete-default' }} id='autocomplete-default' />, scratch)
+      describe('menuAttributes', () => {
+        it('renders with the extra attribute on the menu', () => {
+          render(<Autocomplete menuAttributes={{ 'aria-labelledby': 'autocomplete-default' }} id='autocomplete-default' />, scratch)
 
-        const wrapperElement = scratch.getElementsByClassName('autocomplete__wrapper')[0]
-        const dropdownElement = wrapperElement.getElementsByTagName('ul')[0]
-        const inputElement = wrapperElement.getElementsByTagName('input')[0]
+          const wrapperElement = scratch.getElementsByClassName('autocomplete__wrapper')[0]
+          const dropdownElement = wrapperElement.getElementsByTagName('ul')[0]
+          const inputElement = wrapperElement.getElementsByTagName('input')[0]
 
-        expect(dropdownElement.getAttribute('aria-labelledby')).to.equal('autocomplete-default')
-        expect(inputElement.getAttribute('id')).to.equal('autocomplete-default')
+          expect(dropdownElement.getAttribute('aria-labelledby')).to.equal('autocomplete-default')
+          expect(inputElement.getAttribute('id')).to.equal('autocomplete-default')
+        })
+
+        it('does not override attributes computed by the component', () => {
+          const menuAttributes = {
+            id: 'custom-id',
+            role: 'custom-role',
+            // React uses `className`, but Preact will use `class` as well,
+            // so we need to test for both
+            className: 'custom-class',
+            class: 'other-class'
+          }
+
+          render(<Autocomplete menuAttributes={menuAttributes} id='autocomplete-default' />, scratch)
+
+          // Check that the computed values are the ones expected in the HTML
+          const menuElement = scratch.getElementsByClassName('autocomplete__menu')[0]
+          expect(menuElement.id).to.equal('autocomplete-default__listbox', 'HTML id')
+          expect(menuElement.role).to.equal('listbox', 'HTML role')
+          // Classes get merged rather than discarded
+          expect(menuElement.className).to.equal('autocomplete__menu autocomplete__menu--inline autocomplete__menu--hidden custom-class other-class', 'HTML class')
+
+          // Check that in protecting the menu, we don't affect the object passed as option
+          expect(menuAttributes.id).to.equal('custom-id', 'options id')
+          expect(menuAttributes.role).to.equal('custom-role', 'options role')
+          expect(menuAttributes.className).to.equal('custom-class', 'options class')
+        })
       })
 
       it('renders with extra class on the input', () => {