fix(items): items in list should wrapped in li in case of a

heroui-inc · Nov 29, 2024 · 3d04317 · 3d04317
1 parent ad79545
commit 3d04317
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 34 deletions.
diff --git a/.changeset/nasty-bees-sniff.md b/.changeset/nasty-bees-sniff.md
@@ -0,0 +1,7 @@
+---
+"@nextui-org/listbox": patch
+"@nextui-org/menu": patch
+"@nextui-org/pagination": patch
+---
+
+Fixes missing `<li>` wrapper when `href` prop is passed in `ListboxItem`, `MenuItem`, and `PaginationItem`
diff --git a/packages/components/listbox/src/listbox-item.tsx b/packages/components/listbox/src/listbox-item.tsx
@@ -12,6 +12,7 @@ export interface ListboxItemProps<T extends object = object> extends UseListboxI
 const ListboxItem = forwardRef<"li", ListboxItemProps>((props, _) => {
   const {
     Component,
+    FragmentWrapper,
     rendered,
     description,
     isSelectable,
@@ -22,6 +23,7 @@ const ListboxItem = forwardRef<"li", ListboxItemProps>((props, _) => {
     endContent,
     hideSelectedIcon,
     disableAnimation,
+    fragmentWrapperProps,
     getItemProps,
     getLabelProps,
     getWrapperProps,
@@ -45,19 +47,21 @@ const ListboxItem = forwardRef<"li", ListboxItemProps>((props, _) => {
 
   return (
     <Component {...getItemProps()}>
-      {startContent}
-      {description ? (
-        <div {...getWrapperProps()}>
+      <FragmentWrapper {...fragmentWrapperProps}>
+        {startContent}
+        {description ? (
+          <div {...getWrapperProps()}>
+            <span {...getLabelProps()}>{rendered}</span>
+            <span {...getDescriptionProps()}>{description}</span>
+          </div>
+        ) : (
           <span {...getLabelProps()}>{rendered}</span>
-          <span {...getDescriptionProps()}>{description}</span>
-        </div>
-      ) : (
-        <span {...getLabelProps()}>{rendered}</span>
-      )}
-      {isSelectable && !hideSelectedIcon && (
-        <span {...getSelectedIconProps()}>{selectedContent}</span>
-      )}
-      {endContent}
+        )}
+        {isSelectable && !hideSelectedIcon && (
+          <span {...getSelectedIconProps()}>{selectedContent}</span>
+        )}
+        {endContent}
+      </FragmentWrapper>
     </Component>
   );
 });

diff --git a/packages/components/listbox/src/use-listbox-item.ts b/packages/components/listbox/src/use-listbox-item.ts
@@ -1,6 +1,6 @@
 import type {ListboxItemBaseProps} from "./base/listbox-item-base";
 
-import {useMemo, useRef, useCallback} from "react";
+import {useMemo, useRef, useCallback, Fragment} from "react";
 import {listboxItem} from "@nextui-org/theme";
 import {
   HTMLNextUIProps,
@@ -48,6 +48,7 @@ export function useListboxItem<T extends object>(originalProps: UseListboxItemPr
     shouldHighlightOnFocus,
     hideSelectedIcon = false,
     isReadOnly = false,
+    href,
     ...otherProps
   } = props;
 
@@ -56,9 +57,12 @@ export function useListboxItem<T extends object>(originalProps: UseListboxItemPr
 
   const domRef = useRef<HTMLLIElement>(null);
 
-  const Component = as || (originalProps.href ? "a" : "li");
+  const Component = as || "li";
   const shouldFilterDOMProps = typeof Component === "string";
 
+  const FragmentWrapper = href ? "a" : Fragment;
+  const fragmentWrapperProps = href ? {href} : {};
+
   const {rendered, key} = item;
 
   const isDisabled = state.disabledKeys.has(key) || originalProps.isDisabled;
@@ -165,6 +169,7 @@ export function useListboxItem<T extends object>(originalProps: UseListboxItemPr
 
   return {
     Component,
+    FragmentWrapper,
     domRef,
     slots,
     classNames,
@@ -178,6 +183,7 @@ export function useListboxItem<T extends object>(originalProps: UseListboxItemPr
     selectedIcon,
     hideSelectedIcon,
     disableAnimation,
+    fragmentWrapperProps,
     getItemProps,
     getLabelProps,
     getWrapperProps,

diff --git a/packages/components/menu/src/menu-item.tsx b/packages/components/menu/src/menu-item.tsx
@@ -1,5 +1,6 @@
 import {useMemo, ReactNode} from "react";
 import {forwardRef} from "@nextui-org/system";
+import * as React from "react";
 
 import {UseMenuItemProps, useMenuItem} from "./use-menu-item";
 import {MenuSelectedIcon} from "./menu-selected-icon";
@@ -12,6 +13,7 @@ export interface MenuItemProps<T extends object = object> extends UseMenuItemPro
 const MenuItem = forwardRef<"li", MenuItemProps>((props, _) => {
   const {
     Component,
+    FragmentWrapper,
     slots,
     classNames,
     rendered,
@@ -25,6 +27,7 @@ const MenuItem = forwardRef<"li", MenuItemProps>((props, _) => {
     endContent,
     disableAnimation,
     hideSelectedIcon,
+    fragmentWrapperProps,
     getItemProps,
     getLabelProps,
     getDescriptionProps,
@@ -48,20 +51,22 @@ const MenuItem = forwardRef<"li", MenuItemProps>((props, _) => {
 
   return (
     <Component {...getItemProps()}>
-      {startContent}
-      {description ? (
-        <div className={slots.wrapper({class: classNames?.wrapper})}>
+      <FragmentWrapper {...fragmentWrapperProps}>
+        {startContent}
+        {description ? (
+          <div className={slots.wrapper({class: classNames?.wrapper})}>
+            <span {...getLabelProps()}>{rendered}</span>
+            <span {...getDescriptionProps()}>{description}</span>
+          </div>
+        ) : (
           <span {...getLabelProps()}>{rendered}</span>
-          <span {...getDescriptionProps()}>{description}</span>
-        </div>
-      ) : (
-        <span {...getLabelProps()}>{rendered}</span>
-      )}
-      {shortcut && <kbd {...getKeyboardShortcutProps()}>{shortcut}</kbd>}
-      {isSelectable && !hideSelectedIcon && (
-        <span {...getSelectedIconProps()}>{selectedContent}</span>
-      )}
-      {endContent}
+        )}
+        {shortcut && <kbd {...getKeyboardShortcutProps()}>{shortcut}</kbd>}
+        {isSelectable && !hideSelectedIcon && (
+          <span {...getSelectedIconProps()}>{selectedContent}</span>
+        )}
+        {endContent}
+      </FragmentWrapper>
     </Component>
   );
 });

diff --git a/packages/components/menu/src/use-menu-item.ts b/packages/components/menu/src/use-menu-item.ts
@@ -1,7 +1,7 @@
 import type {MenuItemBaseProps} from "./base/menu-item-base";
 import type {Node} from "@react-types/shared";
 
-import {useMemo, useRef, useCallback} from "react";
+import {useMemo, useRef, useCallback, Fragment} from "react";
 import {menuItem} from "@nextui-org/theme";
 import {
   HTMLNextUIProps,
@@ -57,6 +57,7 @@ export function useMenuItem<T extends object>(originalProps: UseMenuItemProps<T>
     isReadOnly = false,
     closeOnSelect,
     onClose,
+    href,
     ...otherProps
   } = props;
 
@@ -65,9 +66,12 @@ export function useMenuItem<T extends object>(originalProps: UseMenuItemProps<T>
 
   const domRef = useRef<HTMLLIElement>(null);
 
-  const Component = as || (otherProps?.href ? "a" : "li");
+  const Component = as || "li";
   const shouldFilterDOMProps = typeof Component === "string";
 
+  const FragmentWrapper = href ? "a" : Fragment;
+  const fragmentWrapperProps = href ? {href} : {};
+
   const {rendered, key} = item;
 
   const isDisabledProp = state.disabledKeys.has(key) || originalProps.isDisabled;
@@ -190,6 +194,7 @@ export function useMenuItem<T extends object>(originalProps: UseMenuItemProps<T>
 
   return {
     Component,
+    FragmentWrapper,
     domRef,
     slots,
     classNames,
@@ -203,6 +208,7 @@ export function useMenuItem<T extends object>(originalProps: UseMenuItemProps<T>
     endContent,
     selectedIcon,
     disableAnimation,
+    fragmentWrapperProps,
     getItemProps,
     getLabelProps,
     hideSelectedIcon,

diff --git a/packages/components/pagination/src/pagination-item.tsx b/packages/components/pagination/src/pagination-item.tsx
@@ -5,9 +5,14 @@ import {usePaginationItem, UsePaginationItemProps} from "./use-pagination-item";
 export interface PaginationItemProps extends UsePaginationItemProps {}
 
 const PaginationItem = forwardRef<"li", PaginationItemProps>((props, ref) => {
-  const {Component, children, getItemProps} = usePaginationItem({...props, ref});
+  const {Component, FragmentWrapper, fragmentWrapperProps, children, getItemProps} =
+    usePaginationItem({...props, ref});
 
-  return <Component {...getItemProps()}>{children}</Component>;
+  return (
+    <Component {...getItemProps()}>
+      <FragmentWrapper {...fragmentWrapperProps}>{children}</FragmentWrapper>
+    </Component>
+  );
 });
 
 PaginationItem.displayName = "NextUI.PaginationItem";

diff --git a/packages/components/pagination/src/use-pagination-item.ts b/packages/components/pagination/src/use-pagination-item.ts
@@ -2,7 +2,7 @@ import type {Ref} from "react";
 import type {HTMLNextUIProps, PropGetter} from "@nextui-org/system";
 import type {LinkDOMProps, PressEvent} from "@react-types/shared";
 
-import {useMemo} from "react";
+import {Fragment, useMemo} from "react";
 import {PaginationItemValue} from "@nextui-org/use-pagination";
 import {clsx, dataAttr} from "@nextui-org/shared-utils";
 import {chain, mergeProps, shouldClientNavigate, useRouter} from "@react-aria/utils";
@@ -64,10 +64,13 @@ export function usePaginationItem(props: UsePaginationItemProps) {
   } = props;
 
   const isLink = !!props?.href;
-  const Component = as || isLink ? "a" : "li";
+  const Component = as || "li";
   const shouldFilterDOMProps = typeof Component === "string";
-  const domRef = useDOMRef(ref);
 
+  const FragmentWrapper = isLink ? "a" : Fragment;
+  const fragmentWrapperProps = isLink ? {href: props.href} : {};
+
+  const domRef = useDOMRef(ref);
   const router = useRouter();
 
   const ariaLabel = useMemo(
@@ -129,6 +132,8 @@ export function usePaginationItem(props: UsePaginationItemProps) {
 
   return {
     Component,
+    FragmentWrapper,
+    fragmentWrapperProps,
     children,
     ariaLabel,
     isFocused,