carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.86k stars 1.82k forks source link

Allow optional props to be passed to wrapper div, input and label in Search component #7095

Closed gptt916 closed 3 years ago

gptt916 commented 4 years ago

Summary

I would like the Search component to allow specifying optional props to be passed to the wrapper div, the label and the input elements, instead of passing all additional props to the input element

Clarify if you are asking for design, development, or both design and development

development.

Justification

This enables better integration of the component with external libraries such as downshift.js for the purpose of accessibility (combobox). downshift.js requires props to be passed to the elements specified above in order to set aria labels and other attrs for accessibility.

Desired UX and success metrics

Search component takes additional prop(s) and passes them to the correct elements being rendered (wrapper div, label and input)

"Must have" functionality

Allow additional props.

Specific timeline issues / requests

Sooner the better.

Available extra resources

I am available to contribute if this proposal is accepted.

Notes

This enhancement should adequetly provide enough flexibility for users of the Search component to resolve https://github.com/carbon-design-system/carbon/issues/6724

For reference, the downshift.js component I am using is documented here: https://github.com/downshift-js/downshift/tree/master/src/hooks/useCombobox

The example code from the documentation:

import * as React from 'react'
import {render} from 'react-dom'
import {useCombobox} from 'downshift'
// items = ['Neptunium', 'Plutonium', ...]
import {items, menuStyles, comboboxStyles} from './utils'

function DropdownCombobox() {
  const [inputItems, setInputItems] = useState(items)
  const {
    isOpen,
    selectedItem,
    getToggleButtonProps,
    getLabelProps,
    getMenuProps,
    getInputProps,
    getComboboxProps,
    highlightedIndex,
    getItemProps,
  } = useCombobox({
    items: inputItems,
    onInputValueChange: ({inputValue}) => {
      setInputItems(
        items.filter(item =>
          item.toLowerCase().startsWith(inputValue.toLowerCase()),
        ),
      )
    },
  })

  return (
    <>
      <label {...getLabelProps()}>Choose an element:</label>
      <div style={comboboxStyles} {...getComboboxProps()}>
        <input {...getInputProps()} />
        <button
          type="button"
          {...getToggleButtonProps()}
          aria-label={'toggle menu'}
        >
          &#8595;
        </button>
      </div>
      <ul {...getMenuProps()} style={menuStyles}>
        {isOpen &&
          inputItems.map((item, index) => (
            <li
              style={
                highlightedIndex === index ? {backgroundColor: '#bde4ff'} : {}
              }
              key={`${item}${index}`}
              {...getItemProps({item, index})}
            >
              {item}
            </li>
          ))}
      </ul>
    </>
  )
}

render(<DropdownCombobox />, document.getElementById('root'))
tw15egan commented 3 years ago

I don't think we're going to go down the route of {...getLabelProps()}, but we're planning on making elements like this more composable so that optional props can be spread to the correct component

vpicone commented 3 years ago

In the next major version of Carbon, we'll be applying wrapper classNames more consistently. Having a mechanism to pass props through to every single element in a compound component is not something we're looking to support at this time considering how cumbersome of an API that would be to implement and support.

gptt916 commented 3 years ago

Ty for the update, will see what I can do with carbon 11.