Active `NavList.Item` is incorrectly missing bold styling

rwe commented 7 months ago

Description

Active NavList.Items do not have the expected bold styling. Note that this appears to be a bug, diagnosed below, and not browser-specific.

❌ Actual: @primer/react

Compare with expected styling on GitHub's own use of the component:

✅ Expected (via github.com):

And the screenshot from the NavList docs themselves:

✅ Expected (via docs)

Diagnosis

The current/active state of a NavList.Item is controlled by the aria-current property which drives the ActionList.LinkItem → ActionList.Item's active property. That property then decides whether ActionList.Item's isActive styles are applied: https://github.com/primer/react/blob/3ec63098e159d5b9fb14e044e5134972ca8dbfb8/packages/react/src/ActionList/Item.tsx#L125-L138

Those styles are all correctly applied to the container LiBox.

However, (unless a "Description" heading element occurs, which is not desired here and is not shown in the above expected examples), fontWeight is overridden immediately: https://github.com/primer/react/blob/3ec63098e159d5b9fb14e044e5134972ca8dbfb8/packages/react/src/ActionList/Item.tsx#L322

Without being too familiar with the codebase or StyledComponents in general, it seems a possible fix might be something like:

- fontWeight: slots.inlineDescription || slots.blockDescription ? 'bold' : 'normal',
+ fontWeight: slots.inlineDescription || slots.blockDescription ? 'bold' : null,

Steps to reproduce

This is visible in the current storybook: https://primer.style/react/storybook/?path=/story/components-navlist--simple whose code is given as:

export const Simple: Story = () => (
  <PageLayout>
    <PageLayout.Pane position="start">
      <NavList>
        <NavList.Item href="#" aria-current="page">
          Item 1
        </NavList.Item>
        <NavList.Item href="#">Item 2</NavList.Item>
        <NavList.Item href="#">Item 3</NavList.Item>
      </NavList>
    </PageLayout.Pane>
    <PageLayout.Content></PageLayout.Content>
  </PageLayout>
)

It occurs with any simple NavList with a "current" item, and I believe also with a basic ActionList.Item.

Version

36.9.0

Through 369c3036c8701f227ac5e49b38d32f65f6a8ce97 (at time of issue creation).

Browser

(Not browser-specific) Chrome

github-actions[bot] commented 7 months ago

Uh oh! @rwe, the image you shared is missing helpful alt text. Check your issue body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

langermank commented 7 months ago

Thanks for reporting this! I'm going to close this as a duplicate of https://github.com/primer/react/issues/4367, but there's a fix ready to merge that will probably be in the next release.