microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.1k stars 2.69k forks source link

[Feature]: sharing IDREF between compound components #24163

Open bsunderhus opened 1 year ago

bsunderhus commented 1 year ago

Library

React Components / v9 (@fluentui/react-components)

Describe the feature that you would like added

Current behavior

Most of v9 components introduce a default ID by context and passing it down the line to its compound component to consume.

// MenuGroup

export function useMenuGroup_unstable(props: MenuGroupProps, ref: React.Ref<HTMLElement>): MenuGroupState {
  const headerId = useId('menu-group');

  return {
    components: {
      root: 'div',
    },
    root: getNativeElementProps('div', {
      ref,
      'aria-labelledby': headerId, // adds aria attribute with header id
      role: 'group',
      ...props,
    }),
    headerId: headerId, // passes header id to be added through context
  };
}

// MenuGroupHeader

export function useMenuGroupHeader_unstable(
  props: MenuGroupHeaderProps,
  ref: React.Ref<HTMLElement>,
): MenuGroupHeaderState {
  const { headerId: id } = useMenuGroupContext_unstable(); // consumes headerId from MenuGroup provided context

  return {
    components: {
      root: 'div',
    },
    root: getNativeElementProps('div', {
      ref,
      id,
      ...props,
    }),
  };
}

Problem Statement

Some ARIA Patterns require Id references being shared between compound components through Context to ensure functionality, for example: aria-labelledby, aria-describedby, aria-controls, etc,.

In the example usage above of MenuGroup and MenuGroupHeader, If the user provides its own id to MenuGroupHeader, and stops using the one provided by context, then MenuGroup would start pointing aria-labelledby to the wrong place. Current solution is documenting this problem and stating for the user that it is its own job to re-introduce the proper aria-labelledby attribute on its own:

export const GroupingItems = () => (
  <Menu>
    <MenuTrigger>
      <Button>Toggle menu</Button>
    </MenuTrigger>

    <MenuPopover>
      <MenuList>
        {/* aria-labelledby should be re-introduced here, otherwise MenuGroup would point to a unused id */}
        <MenuGroup aria-labelledby="custom-id">
          <MenuGroupHeader id="custom-id">Section header</MenuGroupHeader>
          <MenuItem icon={<CutIcon />}>Cut</MenuItem>
          <MenuItem icon={<PasteIcon />}>Paste</MenuItem>
          <MenuItem icon={<EditIcon />}>Edit</MenuItem>
        </MenuGroup>
        <MenuDivider />
      </MenuList>
    </MenuPopover>
  </Menu>
);

Feature Request

To solve this problem, some sort of context mechanism to add IDREF between compound components is required.

One problem with this is the extra render this would require in the case a child component updates its id reference to its parent, but so far, I can't think of a better solution.

THIS IS STILL OPEN FOR DEBATE

Have you discussed this feature with our team

teams-prague

Additional context

No response

Validations

ling1726 commented 1 year ago

It would be a good issue to solve, it's been tracked for Menu in #18318

ling1726 commented 1 year ago

What would be the source of truth ? the aria-xxx attribute or theid?

bsunderhus commented 1 year ago

What would be the source of truth? the aria-xxx attribute or the id?

I'd assume the id should be the source of truth, as that's the most expected property to be introduced by user.

ling1726 commented 1 year ago

What would be the source of truth? the aria-xxx attribute or the id?

I'd assume the id should be the source of truth, as that's the most expected property to be introduced by user.

In that case I think the forced render is fine.

Perhaps there might also be some way to do it on DOM with ref ? you could track the id through a ref and calling some kind of setUserId callback in context could just update that directly in DOM

msft-fluent-ui-bot commented 1 year ago

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

msft-fluent-ui-bot commented 1 year ago

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

microsoft-github-policy-service[bot] commented 6 months ago

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

microsoft-github-policy-service[bot] commented 1 month ago

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.