shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
73.72k stars 4.54k forks source link

[bug]: SheetTitle Error on Sidebar #5474

Open bearbricknik opened 2 weeks ago

bearbricknik commented 2 weeks ago

Describe the bug

Hi everyone,

I found a small bug in the mobile view of the sidebars. The mobile sidebar is build on top of the sheet component and therefore requires a SheetTitle.

The error is: DialogContent requires a DialogTitle for the component to be accessible for screen reader users.

If you want to hide the DialogTitle, you can wrap it with our VisuallyHidden component.

For more information, see https://radix-ui.com/primitives/docs/components/dialog

To resolve the issue, you can either add the SheetTitle or use the Radix VisuallyHidden component to hide the Sheet Title.

`

Menu
        </VisuallyHidden.Root>`

In particular: The sidebar.tsx has the following code: ` if (isMobile) { return ( <Sheet open={openMobile} onOpenChange={setOpenMobile} {...props}> <SheetContent data-sidebar="sidebar" data-mobile="true" className="w-[--sidebar-width] bg-sidebar p-0 text-sidebar-foreground [&>button]:hidden" style={ { "--sidebar-width": SIDEBAR_WIDTH_MOBILE, } as React.CSSProperties } side={side}

Menu
{children}
) }`
Just add it above the div to visually hide it but fix the sheet error,
screeshot

Affected component/components

sidebar.tsx

How to reproduce

  1. Use mobile sidebar

Codesandbox/StackBlitz link

No response

Logs

No response

System Info

-

Before submitting

TJdhyani commented 2 weeks ago

Facing the same issue

TJdhyani commented 2 weeks ago

Install @radix-ui/react-visually-hidden & Add this in your sidebar.tsx:

    if (isMobile) {
      return (
        <Sheet open={openMobile} onOpenChange={setOpenMobile} {...props}>
          <SheetContent
            data-sidebar="sidebar"
            data-mobile="true"
            className="w-[--sidebar-width] bg-sidebar p-0 text-sidebar-foreground [&>button]:hidden"
            style={
              {
                '--sidebar-width': SIDEBAR_WIDTH_MOBILE,
              } as React.CSSProperties
            }
            side={side}
          >
++++        <VisuallyHidden>
++++          <SheetHeader>
++++            <SheetTitle>Sidebar</SheetTitle>
++++            <SheetDescription>Sidebar</SheetDescription>
++++          </SheetHeader>
++++        </VisuallyHidden>
            <div className="flex h-full w-full flex-col">{children}</div>
          </SheetContent>
        </Sheet>
      )
    }

Solution: https://github.com/shadcn-ui/ui/issues/4302#issuecomment-2423790230

hellocory commented 1 week ago

Hi. I fixed this issue by adding:

<DialogTitle className="sr-only">Sheet</DialogTitle>

Put this inside the sheet content to fix this. When trying to add the description, it won't work unless there is a title.

Medaillek commented 1 week ago

Hi. I fixed this issue by adding:

<DialogTitle className="sr-only">Sheet</DialogTitle>

Put this inside the sheet content to fix this. When trying to add the description, it won't work unless there is a title.

Did the same, easier than wrapping into another component you need to install, here you have full control using sr-only

AhmadYousif89 commented 1 week ago

Hi. I fixed this issue by adding:

<DialogTitle className="sr-only">Sheet</DialogTitle>

Put this inside the sheet content to fix this. When trying to add the description, it won't work unless there is a title.

That did the trick, and to be more precise with this solution here is where you should add the code snippet:

const SheetContent = React.forwardRef<
  React.ElementRef<typeof SheetPrimitive.Content>,
  SheetContentProps
>(({ side = "right", className, children, ...props }, ref) => (
  <SheetPortal>
    <SheetOverlay />
    <SheetPrimitive.Content
      ref={ref}
      className={cn(sheetVariants({ side }), className)}
      {...props}
    >
++++      <SheetPrimitive.DialogTitle className="sr-only">
++++         Sheet
++++      </SheetPrimitive.DialogTitle>
      {children}
      <SheetPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-secondary">
        <X className="h-4 w-4" />
        <span className="sr-only">Close</span>
      </SheetPrimitive.Close>
    </SheetPrimitive.Content>
  </SheetPortal>
))
SheetContent.displayName = SheetPrimitive.Content.displayName
FlashAmarillo commented 6 days ago

Install @radix-ui/react-visually-hidden & Add this in your sidebar.tsx:

    if (isMobile) {
      return (
        <Sheet open={openMobile} onOpenChange={setOpenMobile} {...props}>
          <SheetContent
            data-sidebar="sidebar"
            data-mobile="true"
            className="w-[--sidebar-width] bg-sidebar p-0 text-sidebar-foreground [&>button]:hidden"
            style={
              {
                '--sidebar-width': SIDEBAR_WIDTH_MOBILE,
              } as React.CSSProperties
            }
            side={side}
          >
++++        <VisuallyHidden>
++++          <SheetHeader>
++++            <SheetTitle>Sidebar</SheetTitle>
++++            <SheetDescription>Sidebar</SheetDescription>
++++          </SheetHeader>
++++        </VisuallyHidden>
            <div className="flex h-full w-full flex-col">{children}</div>
          </SheetContent>
        </Sheet>
      )
    }

Solution: #4302 (comment)

Thank you so much 🫂