UnderlineNav2 children changes not reflected

[jeffwilcox]

Description

For a long time, we've been using the original UnderlineNav where state would dictate the visible underlined items, to essentially lightup a few additional items for administrators, etc.

Recently, we noticed this regressed, but haven't worried about it too much, since with cached state and other items, a refresh of the page, or if the component tree shifts enough, would fix the glitch.

Any chance this is as expected? At least today, this all behaves differently than basic React lists, for example... it's possible this is a regression in the "new" UnderlineNav, or, maybe related to some of the changes such as #3559 ?

Steps to reproduce

1. Add this sample component to a simple Primer React sandbox
2. Wait until the page renders, and after 2 seconds, the state will flip.
3. Expected: an item is added to the first UnderlineNav, one is removed from the 2nd; and the 3rd has an item added. Equivalent behavior on the same state for a <ul /> is also shown to observe.
4. Actual: no change to the children in UnderlineNav

import { useEffect, useState } from 'react';
import { Box, Text, UnderlineNav } from '@primer/react';

// The state change doesn't render the updated list.
// This was definitely working for us in the past, but it's possible
// we were refreshing the render tree higher up in the component tree,
// so maybe it's by design?

function getItems(state: boolean) {
  const items: string[] = [
    'Item 1',
    'Item 2',
    'Item 3'
  ];
  if (state === true) {
    items.push('Item 4');
  }
  return items;
}

export const UnderlineNavBugRepro = () => {
  const [someState, setSomeState] = useState(false);
  const [someState2, setSomeState2] = useState(true);
  useEffect(() => {
    const timer = setTimeout(() => {
      setSomeState(true);
      setSomeState2(false);
    }, 1000);
    return () => clearTimeout(timer);
  }, []);

  const runtimeItems = getItems(someState);

  return (
    <Box>
      <Text as="p">
        The state is {String(someState)}.
      </Text>

      <Text as="p">
        Given the state right now, there should be visible {someState === true ? '4' : '3'} items in the UnderlineNav.
      </Text>
      <UnderlineNav aria-label="Main">
        <UnderlineNav.Item
          onSelect={(e: any) => {
            e.preventDefault();
          }}
        >Item 1</UnderlineNav.Item>
        <UnderlineNav.Item
          onSelect={(e: any) => {
          e.preventDefault();
        }}
        >Item 2</UnderlineNav.Item>
        <UnderlineNav.Item
          onSelect={(e: any) => {
            e.preventDefault();
          }}
        >Item 3</UnderlineNav.Item>
        {someState === true && (
          <UnderlineNav.Item
            onSelect={(e: any) => {
              e.preventDefault();
            }}
          >Item 4</UnderlineNav.Item>
        )}
      </UnderlineNav>

      <Text as="p">Inverse:</Text>
      <Text as="p">
        Given the state right now, there should be visible {someState === true ? '3' : '4'} items in the UnderlineNav.
      </Text>

      <UnderlineNav aria-label="Main">
        <UnderlineNav.Item
          onSelect={(e: any) => {
            e.preventDefault();
          }}
        >Item 1</UnderlineNav.Item>
        <UnderlineNav.Item
          onSelect={(e: any) => {
          e.preventDefault();
        }}
        >Item 2</UnderlineNav.Item>
        <UnderlineNav.Item
          onSelect={(e: any) => {
            e.preventDefault();
          }}
        >Item 3</UnderlineNav.Item>
        {someState2 === true && (
          <UnderlineNav.Item
            onSelect={(e: any) => {
              e.preventDefault();
            }}
          >Item 4</UnderlineNav.Item>
        )}
      </UnderlineNav>

      <Text as="p">
        Items map:
      </Text>
      <Text as="p">
        Given the state right now, there should be visible {someState === true ? '4' : '3'} items in the UnderlineNav.
      </Text>
      <UnderlineNav aria-label='Items'>
        {runtimeItems.map((item: string, index: number) => (
          <UnderlineNav.Item
            key={index}
            onSelect={(e: any) => {
              e.preventDefault();
            }}
          >{item}</UnderlineNav.Item>
        ))}
      </UnderlineNav>

      <Text as="p">
        Standard unordered list, inline:
      </Text>
      <Text as="p">
        Given the state right now, there should be visible {someState === true ? '4' : '3'} items in the UnderlineNav.
      </Text>
      <ul>
        <li>Item 1</li>
        <li>Item 2</li>
        <li>Item 3</li>
        {someState === true && (
          <li>Item 4</li>
        )}
      </ul>

      <Text as="p">
        Standard unordered list, items map:
      </Text>
      <Text as="p">
        Given the state right now, there should be visible {someState === true ? '4' : '3'} items in the UnderlineNav.
      </Text>
      <ul>
        {runtimeItems.map((item: string, index: number) => (
          <li key={index}>{item}</li>
        ))}
      </ul>
    </Box>
  );
}

Version

v.36.5.0

Browser

Edge

[jeffwilcox]

Here's a short repro vid

underline-nav-short-video.mov

[lesliecdubs]

Hi @jeffwilcox 👋🏻 Thanks for filing this. It sounds like you've been able to work around this for now, but if that changes please let us know.

@broccolinisoup when you're back from vacation, curious if you can think of any recent changes to UnderlineNav that might have introduced this behavior?

[broccolinisoup]

Hello @jeffwilcox 👋

Sorry for a very late response!

I can confirm that the behaviour you are seeing is due to design and it is unfortunate not to be able use the component with the traditional React mindset 😢 I am now thinking maybe we can tweak this behaviour since you are experiencing an issue. 🤔

The component actually gets re-rendered when the state changes, i.e. when the number of the children is changed, but it doesn't update the final list that makes up the inline and menu items and this is something we had to sacrifice to accomodate one of the overflow features of UnderlineNav. We only update the list and menu items when resize is triggered.

Could you point me the code where this behaviour is happening? I'd like to understand the use case better and see if we can accomodate it. Thank you 🙏

[jeffwilcox]

Cool, appreciate the response in any form!

... the code is not pretty, that's for sure, but I think I could point you at a few examples. It's typically in a few rare spaces where we light up additional items only for administrators / feature flagged users / members of a resource which general users may not, yet that state is usually being loaded async.

One change at a cost of responsiveness we've considered is waiting to have answers on those flags before rendering the parent component that contains UnderlineNav2.

Since the project is half open source and half in the Microsoft internal EMU instance (I'm a Microsoftie), while you should have creds for it, I am definitely not expecting any support beyond the response here same as any oss participant.

[github-actions]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.