JedWatson / JedWatson/react-select

formatGroupLabel returns string but not used that way in documentation

Open
#4,407 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

category/accessibility category/documentation issue/needs-review
Dominant language
TypeScript
Stars
28k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

Perhaps a breaking change for some so wanted to document it with a possibility to discuss/resolve this later.

Given these props, there are different expectations about the return type.
getOptionLabel => string
getOptionValue => string
formatOptionLabel => react.Node
formatGroupLabel => string

The nomenclature suggests that formatGroupLabel would likely be expected to render a react Node as well, but instead it is defined to return a string. I say likely expected because the first grouping example on the homepage of the documentation misuses this prop in this exact way.

const formatGroupLabel = data => (
  <div style={groupStyles}>
    <span>{data.label}</span>
    <span style={groupBadgeStyles}>{data.options.length}</span>
  </div>
);

export default () => (
  <Select
    defaultValue={colourOptions[1]}
    options={groupedOptions}
    formatGroupLabel={formatGroupLabel}
  />
);

While it might not be a big deal to some, misusing these props has an impact on accessibility as it is reliant to relay the label back to the screen reader and will instead return "object object" as the selected option.

Recommendations:

  1. Add group as a possible context for formatOptionLabel to consolidate the render functions there.
  2. Remove/replace the code example shown above (and possibly other examples) in the documentation to convey best practices.
  3. Consider adding documentation about accessibility concerns when using getOptionLabel and formatGroupLabel
  4. Consider renaming formatGroupLabel to getGroupLabel in a future major release.
  5. Consider adding console warnings when an object is stringified.

I could create a PR, but wanted to get thoughts on listed considerations before moving forward

Let it also be noted that the accessibility concerns around the current stringification has been noted here: https://github.com/JedWatson/react-select/issues/4134

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the grouping example on the documentation homepage and review the related accessibility discussion in issue 4134. Determine which of the listed recommendations are accepted, then update the affected documentation and examples so the agreed behavior and accessibility guidance are clear.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
accessibility, documentation, frontend
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.