fix: add default aria-label to popover button and fix double-hyphen bug

packages/demo-html/public/popover-html.html

@@ -14,6 +14,7 @@
       id="button"
       data-tf-notification-days="3"
       data-tf-enable-sandbox
+      data-tf-button-props="ariaLabel=Typeform Button"
     ></div>
     <script src="./lib/embed.js"></script>
   </body>

packages/demo-html/public/popover-js.html

@@ -35,6 +35,7 @@
           foo: 'foo value',
           bar: 'bar value',
         },
+        buttonProps: { ariaLabel: 'Typeform Button' },
       })
 
       document.getElementById('button').onclick = toggle
@@ -47,6 +48,7 @@
             foo: 'foo value',
             bar: 'bar value',
           },
+          buttonProps: { ariaLabel: 'Typeform Button' },
           customIcon: 'https://images.typeform.com/images/dUMgPyHJHM4B',
         })
       }

packages/embed/e2e/functional/popover.test.ts

@@ -16,6 +16,7 @@ for (const [title, path] of pages) {
     const openIcon = button.getByTestId('tf-v1-popover-button-icon')
 
     await expect(button, 'button displayed on page load').toBeVisible()
+    await expect(button).toHaveAttribute('aria-label', 'Typeform Button')
     await expect(defaultIcon).toBeVisible()
     await expect(wrapper, 'popover not displayed on page load').toHaveCount(0)
 

packages/embed/src/factories/create-popover/create-popover.spec.ts

@@ -48,6 +48,14 @@ describe('#createPopover', () => {
         iframe?.onload?.({ isTrusted: true } as Event)
         expect(screen.getByTestId('tf-v1-popover-button-icon')).toBeInTheDocument()
       })
+
+      it('should mark the trigger button as expanded', () => {
+        const button = screen.getByTestId('tf-v1-popover-button')
+        expect(button).toHaveAttribute('aria-expanded', 'false')
+        popover.open()
+        jest.runAllTimers()
+        expect(button).toHaveAttribute('aria-expanded', 'true')
+      })
     })
 
     describe('#close', () => {
@@ -71,6 +79,16 @@ describe('#createPopover', () => {
         expect(screen.getByTestId('default-icon')).toBeInTheDocument()
       })
 
+      it('should mark the trigger button as collapsed', async () => {
+        const button = screen.getByTestId('tf-v1-popover-button')
+        popover.open()
+        jest.runAllTimers()
+        screen.getByTestId('iframe')?.onload?.({ isTrusted: true } as Event)
+        popover.close()
+        expect(button).toHaveAttribute('aria-expanded', 'false')
+        await waitForElementToBeRemoved(() => screen.queryByTestId('tf-v1-popover-wrapper'))
+      })
+
       it('should run onClose callback if provided', () => {
         const onClose = jest.fn()
         const popover = createPopover('formId', { onClose })
@@ -185,5 +203,31 @@ describe('#createPopover', () => {
         expect(screen.getByTitle('pop')).toHaveAttribute('aria-label', 'foo')
       })
     })
+
+    describe('#default aria-label', () => {
+      it('should have a default aria-label for accessibility', () => {
+        popover = createPopover('formId')
+        const button = screen.getByTestId('tf-v1-popover-button')
+        expect(button).toHaveAttribute('aria-label', 'Open form')
+      })
+
+      it('should allow overriding the default aria-label', () => {
+        popover = createPopover('formId', { buttonProps: { ariaLabel: 'Custom label' } })
+        const button = screen.getByTestId('tf-v1-popover-button')
+        expect(button).toHaveAttribute('aria-label', 'Custom label')
+      })
+
+      it('should allow overriding the default aria-label with a hyphenated key (data-tf-button-props)', () => {
+        popover = createPopover('formId', { buttonProps: { 'aria-label': 'Custom label' } as never })
+        const button = screen.getByTestId('tf-v1-popover-button')
+        expect(button).toHaveAttribute('aria-label', 'Custom label')
+      })
+
+      it('should use the default aria-label when ariaLabel is undefined', () => {
+        popover = createPopover('formId', { buttonProps: { ariaLabel: undefined } })
+        const button = screen.getByTestId('tf-v1-popover-button')
+        expect(button).toHaveAttribute('aria-label', 'Open form')
+      })
+    })
   })
 })

packages/embed/src/factories/create-popover/create-popover.ts

@@ -78,6 +78,8 @@ const buildIcon = (customIcon?: string, color?: string) => {
   return triggerIcon
 }
 
+const DEFAULT_TRIGGER_ARIA_LABEL = 'Open form'
+
 const buildTriggerButton = (color: string, buttonProps: ButtonProps = {}) => {
   const textColor = getTextColor(color)
   const button = document.createElement('button')
@@ -86,6 +88,12 @@ const buildTriggerButton = (color: string, buttonProps: ButtonProps = {}) => {
   button.style.backgroundColor = color
   button.style.color = textColor
   addAttributesToElement(button, buttonProps)
+  // Icon-only button needs an accessible name. Check the DOM so any key form the user passed counts.
+  if (!button.hasAttribute('aria-label')) {
+    button.setAttribute('aria-label', DEFAULT_TRIGGER_ARIA_LABEL)
+  }
+  // The label stays constant, open state is conveyed through aria-expanded
+  button.setAttribute('aria-expanded', 'false')
   return button
 }
 
@@ -182,6 +190,7 @@ export const createPopover = (formId: string, userOptions: PopoverOptions = {}):
 
   const open = () => {
     if (!isOpen(wrapper)) {
+      button.setAttribute('aria-expanded', 'true')
       hideTooltip()
       hideNotificationDot()
       setTimeout(() => {
@@ -207,6 +216,7 @@ export const createPopover = (formId: string, userOptions: PopoverOptions = {}):
 
   const close = () => {
     if (isOpen(popover)) {
+      button.setAttribute('aria-expanded', 'false')
       handlePreventReopenOnClose(options, formId)
       userOptions.onClose?.()
       setTimeout(() => {

packages/embed/src/utils/add-attributes-to-element.spec.ts

@@ -19,6 +19,24 @@ describe('#addAttributesToElement', () => {
     expect(element.getAttribute('data-custom-value')).toBe('bar')
   })
 
+  it('should not double-hyphenate attributes that already contain hyphens', () => {
+    const element = document.createElement('button')
+    addAttributesToElement(element, {
+      'aria-label': 'test',
+      'data-custom-id': '123',
+    })
+    expect(element.getAttribute('aria-label')).toBe('test')
+    expect(element.getAttribute('data-custom-id')).toBe('123')
+  })
+
+  it('should skip null and undefined values', () => {
+    const element = document.createElement('button')
+    addAttributesToElement(element, { title: undefined, 'data-foo': null, 'aria-label': 'ok' })
+    expect(element.hasAttribute('title')).toBe(false)
+    expect(element.hasAttribute('data-foo')).toBe(false)
+    expect(element.getAttribute('aria-label')).toBe('ok')
+  })
+
   it('should set element style from a string', () => {
     const element = document.createElement('button')
     addAttributesToElement(element, {

packages/embed/src/utils/add-attributes-to-element.ts

@@ -2,6 +2,9 @@ import { camelCaseToKebabCase } from './load-options-from-attributes'
 
 export const addAttributesToElement = (element: HTMLElement, props = {}) => {
   Object.keys(props).forEach((key) => {
+    if (props[key] === undefined || props[key] === null) {
+      return
+    }
     element.setAttribute(camelCaseToKebabCase(key), props[key])
   })
 }

packages/embed/src/utils/load-options-from-attributes.ts

@@ -2,7 +2,8 @@ export const camelCaseToKebabCase = (value: string) => {
   return value
     .split('')
     .map((letter, i) => {
-      if (letter.toUpperCase() === letter) {
+      // Only add hyphen before uppercase letters (A-Z), not other characters like hyphens or numbers
+      if (letter >= 'A' && letter <= 'Z') {
         return `${i !== 0 ? '-' : ''}${letter.toLowerCase()}`
       }
       return letter