feat(Nav): Added default nav item wrapper. Added support for nav item icons by tlabaj · Pull Request #10687

feat(Nav): Added default nav item wrapper. Added support for nav item icons by tlabaj · Pull Request #10687 · patternfly/patternfly-react

Conversation

This was linked to issues

Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

B-E-A-uuuuutiful!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, but other than that looks great!

		>
		{hasNavLinkWrapper ? <span className={css(`${styles.nav}__link-text`)}>{children}</span> : children}
		{flyout && flyoutButton}
		{icon && <span className={css(`${styles.nav}__link-icon`)}>{icon}</span>}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

	{icon && <span className={css(`${styles.nav}__link-icon`)}>{icon}</span>}
	{icon && <span className={css(styles.navLinkIcon)}>{icon}</span>}

		{...props}
		>
		{hasNavLinkWrapper ? <span className={css(`${styles.nav}__link-text`)}>{children}</span> : children}
		{flyout && flyoutButton}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be added back in. Right now the Flyout example doesn't render the icon to indicate the item has any sort of interaction (item "Flyout link 3" in the following screenshot):