Skip to content

UnderlineNav2: Handle the case when container is too small to render any items #2770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gentle-queens-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

UnderlineNav2: Handle the case when container is too small to render any items
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 20 additions & 4 deletions src/UnderlineNav2/UnderlineNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export type UnderlineNavProps = {
// When page is loaded, we don't have ref for the more button as it is not on the DOM yet.
// However, we need to calculate number of possible items when the more button present as well. So using the width of the more button as a constant.
const MORE_BTN_WIDTH = 86
// The height is needed to make sure we don't have a layout shift when the more button is the only item in the nav.
const MORE_BTN_HEIGHT = 45

// Needed this because passing a ref using HTMLULListElement to `Box` causes a type error
const NavigationList = styled.ul`
Expand All @@ -43,6 +45,8 @@ const NavigationList = styled.ul`

const MoreMenuListItem = styled.li`
display: flex;
align-items: center;
height: ${MORE_BTN_HEIGHT}px;
`

const overflowEffect = (
Expand Down Expand Up @@ -94,7 +98,8 @@ const overflowEffect = (
const ariaCurrent = child.props['aria-current']
const isCurrent = Boolean(ariaCurrent) && ariaCurrent !== 'false'
// We need to make sure to keep the selected item always visible.
if (isCurrent) {
// To do that, we swap the selected item with the last item in the list to make it visible. (When there is at least 1 item in the list to swap.)
if (isCurrent && numberOfListItems > 0) {
// If selected item couldn't make in to the list, we swap it with the last item in the list.
const indexToReplaceAt = numberOfListItems - 1 // because we are replacing the last item in the list
// splice method modifies the array by removing 1 item here at the given index and replace it with the "child" element then returns the removed item.
Expand Down Expand Up @@ -251,6 +256,8 @@ export const UnderlineNav = forwardRef(
}, [])

const actions = responsiveProps.actions
// This is the case where the viewport is too narrow to show any list item with the more menu. In this case, we only show the dropdown
const onlyMenuVisible = responsiveProps.items.length === 0
const [childWidthArray, setChildWidthArray] = useState<ChildWidthArray>([])
const setChildrenWidth = useCallback((size: ChildSize) => {
setChildWidthArray(arr => {
Expand Down Expand Up @@ -336,7 +343,7 @@ export const UnderlineNav = forwardRef(
{responsiveProps.items}
{actions.length > 0 && (
<MoreMenuListItem ref={moreMenuRef}>
<Box sx={getDividerStyle(theme)}></Box>
{!onlyMenuVisible && <Box sx={getDividerStyle(theme)}></Box>}
<Button
ref={moreMenuBtnRef}
sx={moreBtnStyles}
Expand All @@ -346,7 +353,15 @@ export const UnderlineNav = forwardRef(
trailingAction={TriangleDownIcon}
>
<Box as="span">
More<VisuallyHidden as="span">&nbsp;{`${ariaLabel} items`}</VisuallyHidden>
{onlyMenuVisible ? (
<>
<VisuallyHidden as="span">{`${ariaLabel}`}&nbsp;</VisuallyHidden>Menu
</>
) : (
<>
More<VisuallyHidden as="span">&nbsp;{`${ariaLabel} items`}</VisuallyHidden>
</>
)}
</Box>
</Button>
<ActionList
Expand All @@ -365,7 +380,8 @@ export const UnderlineNav = forwardRef(
as={action.props.as || 'a'}
sx={menuItemStyles}
onSelect={(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
swapMenuItemWithListItem(action, index, event, updateListAndMenu)
// When there are no items in the list, do not run the swap function as we want to keep everything in the menu.
!onlyMenuVisible && swapMenuItemWithListItem(action, index, event, updateListAndMenu)
setSelectEvent(event)
closeOverlay()
focusOnMoreMenuBtn()
Expand Down
9 changes: 8 additions & 1 deletion src/UnderlineNav2/UnderlineNavItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,14 @@ export const UnderlineNavItem = forwardRef(
setChildrenWidth({text, width: domRect.width})
setNoIconChildrenWidth({text, width: domRect.width - iconWidthWithMargin})

if (selectedLink === undefined && Boolean(ariaCurrent) && ariaCurrent !== 'false') {
// When an item has aria-current !== false while rendering, we should be sure to select it.
// It can happen when the page is loaded (selectedLink === undefined)
// or if the item is coming out of the menu when there is enough space to show items along with the more menu. (selectedLink.current === null)
if (
(selectedLink === undefined || selectedLink.current === null) &&
Boolean(ariaCurrent) &&
ariaCurrent !== 'false'
) {
setSelectedLink(ref as RefObject<HTMLElement>)
}

Expand Down
1 change: 1 addition & 0 deletions src/UnderlineNav2/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export const getDividerStyle = (theme?: Theme) => ({
width: '1px',
borderLeftColor: `${theme?.colors.border.muted}`,
marginRight: 1,
height: '24px', // The height of the divider - reference from Figma
})

export const moreBtnStyles = {
Expand Down