Skip to content

Commit

Permalink
UnderlineNav2: Handle the case when container is too small to render …
Browse files Browse the repository at this point in the history
…any items (#2770)

* handle the case when container is too small to render any items

* add changeset

* update snapshot

* implement design feedback

* update snapshots and add comments

* test(vrt): update snapshots

* test(vrt): update snapshots

* Use Menu as the button text

* test(vrt): update snapshots

---------

Co-authored-by: broccolinisoup <broccolinisoup@users.noreply.github.com>
Co-authored-by: Josh Black <joshblack@github.com>
  • Loading branch information
3 people authored Feb 2, 2023
1 parent ca7398d commit 3bf1b0e
Show file tree
Hide file tree
Showing 19 changed files with 34 additions and 5 deletions.
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

0 comments on commit 3bf1b0e

Please sign in to comment.