-
-
Notifications
You must be signed in to change notification settings - Fork 45
Menu Component #31
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
Menu Component #31
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Avatar, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Badge, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Button, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Menu, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Tabs, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TabsContent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TabsPanels, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -99,6 +100,21 @@ export default function page() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Alert> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Menu> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Menu.Trigger asChild> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Button>Menu</Button> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Menu.Trigger> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Menu.Content className="min-w-36"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Menu.Item>Menu Item 1</Menu.Item> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Menu.Item>Menu Item 2</Menu.Item> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Menu.Item>Menu Item 3</Menu.Item> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Menu.Content> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Menu> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+104
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance Menu accessibility and user experience While the basic implementation works, consider these improvements:
Here's a suggested implementation: <Menu>
<Menu.Trigger asChild>
- <Button>Menu</Button>
+ <Button aria-label="Open menu options">Menu</Button>
</Menu.Trigger>
- <Menu.Content className="min-w-36">
+ <Menu.Content className="min-w-36" aria-label="Menu options">
- <Menu.Item>Menu Item 1</Menu.Item>
- <Menu.Item>Menu Item 2</Menu.Item>
- <Menu.Item>Menu Item 3</Menu.Item>
+ <Menu.Item>
+ <span className="font-medium">Menu Item 1</span>
+ <span className="text-sm text-gray-500 ml-2">Description</span>
+ </Menu.Item>
+ <Menu.Item>
+ <span className="font-medium">Menu Item 2</span>
+ <span className="text-sm text-gray-500 ml-2">Description</span>
+ </Menu.Item>
+ <Menu.Item>
+ <span className="font-medium">Menu Item 3</span>
+ <span className="text-sm text-gray-500 ml-2">Description</span>
+ </Menu.Item>
</Menu.Content>
</Menu>📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="h-36"></div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,14 +26,19 @@ export const componentConfig = { | |
| name: "card", | ||
| filePath: "packages/ui/Cards/Card.tsx", | ||
| }, | ||
| text: { | ||
| name: "text", | ||
| filePath: "packages/ui/Text/Text.tsx", | ||
| }, | ||
|
|
||
| dialog: { | ||
| name: "dialog", | ||
| filePath: "packages/ui/Dialog/Dialog.tsx", | ||
| }, | ||
| menu: { | ||
| name: "menu", | ||
| filePath: "packages/ui/Menu/Menu.tsx", | ||
| }, | ||
| text: { | ||
| name: "text", | ||
| filePath: "packages/ui/Text/Text.tsx", | ||
| }, | ||
| }, | ||
| examples: { | ||
| "accordion-style-default": { | ||
|
|
@@ -134,6 +139,11 @@ export const componentConfig = { | |
| filePath: "preview/components/input-style-default.tsx", | ||
| preview: lazy(() => import("@/preview/components/input-style-default")), | ||
| }, | ||
| "menu-style-default": { | ||
| name: "menu-style-default", | ||
| filePath: "preview/components/menu-style-default.tsx", | ||
| preview: lazy(() => import("@/preview/components/menu-style-default")), | ||
| }, | ||
|
Comment on lines
+142
to
+146
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Found several name-path mismatches in component examples Several components have inconsistent name properties that don't match their file paths:
These mismatches could cause confusion and should be corrected to maintain consistency throughout the codebase. 🔗 Analysis chainLGTM! New menu example follows the established pattern. The configuration is correctly structured with proper lazy loading implementation. I noticed some examples might have incorrect name properties. Let's verify: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for mismatches between example names and their file paths
# Extract examples with potential name mismatches
rg -A 2 'name: ".+?".*\n.*filePath: "preview/components/(.+?)\.tsx"' | \
grep -E 'name:|filePath:' | \
awk '{
if($1=="name:") name=substr($2,2,length($2)-3);
if($1=="filePath:") {
split($2,path,"/");
file=substr(path[3],1,length(path[3])-5);
if(name!=file) printf "Mismatch - Name: %s, File: %s\n",name,file;
}
}'
Length of output: 539 Script: #!/bin/bash
# Let's try a different approach using ripgrep's multiline mode
rg -U --multiline '^.*"name": "([^"]+)".*\n.*"filePath": "preview/components/([^"]+)\.tsx"' config/components.ts | \
while IFS= read -r line; do
name=$(echo "$line" | grep -o '"name": "[^"]*"' | cut -d'"' -f4)
file=$(echo "$line" | grep -o '"filePath": "[^"]*"' | cut -d'"' -f4 | awk -F'/' '{print $NF}' | sed 's/\.tsx$//')
if [ "$name" != "$file" ]; then
echo "Mismatch - Name: $name, File: $file"
fi
done
Length of output: 136 Script: #!/bin/bash
# Let's simplify and try a different approach
rg -A1 'name:' config/components.ts | \
grep -E '(name:|filePath:)' | \
sed 's/[",]//g' | \
sed 's/^ *//'
Length of output: 2736 |
||
| "textarea-style-default": { | ||
| name: "textarea-style-default", | ||
| filePath: "preview/components/textarea-style-default.tsx", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| --- | ||
| title: Menu | ||
| description: Show your users a list of actions they can take. 📋 | ||
| lastUpdated: 19 Oct, 2024 | ||
| links: | ||
| api_reference: https://www.radix-ui.com/primitives/docs/components/dropdown-menu#api-reference | ||
| source: https://github.com/Logging-Stuff/RetroUI/blob/main/packages/ui/Menu/Menu.tsx | ||
| --- | ||
|
|
||
| <ComponentShowcase name="menu-style-default" /> | ||
| <br /> | ||
| <br /> | ||
|
|
||
| ## Installation | ||
|
|
||
| #### 1. Install dependencies: | ||
|
|
||
| ```sh | ||
| npm install @radix-ui/react-dropdown-menu | ||
| ``` | ||
|
|
||
| <br /> | ||
|
|
||
| #### 2. Copy the code 👇 into your project: | ||
|
|
||
| <ComponentSource name="menu" /> | ||
|
|
||
| <br /> | ||
| <br /> | ||
|
|
||
| ## Examples | ||
|
|
||
| ### Default | ||
|
|
||
| <ComponentShowcase name="menu-style-default" /> | ||
|
Comment on lines
+31
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance the examples section. Consider adding:
Would you like me to help generate additional example code and documentation for common use cases? |
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,7 @@ import { cn } from "@/lib/utils"; | |||||
| import { cva, VariantProps } from "class-variance-authority"; | ||||||
| import React, { ButtonHTMLAttributes } from "react"; | ||||||
|
|
||||||
| const buttonVariants = cva("font-head transition-all", { | ||||||
| const buttonVariants = cva("font-head transition-all outline-none", { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ensure focus visibility for accessibility. While removing the default outline, make sure to maintain a visible focus indicator for keyboard navigation. Consider adding a custom focus style using -const buttonVariants = cva("font-head transition-all outline-none", {
+const buttonVariants = cva("font-head transition-all outline-none focus-visible:ring-2 focus-visible:ring-primary-400 focus-visible:ring-offset-2", {📝 Committable suggestion
Suggested change
|
||||||
| variants: { | ||||||
| variant: { | ||||||
| default: | ||||||
|
|
@@ -39,6 +39,7 @@ export const Button = React.forwardRef<HTMLButtonElement, IButtonProps>( | |||||
| forwardedRef | ||||||
| ) => ( | ||||||
| <button | ||||||
| ref={forwardedRef} | ||||||
| className={cn(buttonVariants({ variant, size }), className)} | ||||||
| {...props} | ||||||
| > | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| "use client"; | ||
|
|
||
| import { cn } from "@/lib/utils"; | ||
| import * as DropdownMenu from "@radix-ui/react-dropdown-menu"; | ||
| import React, { ComponentPropsWithoutRef, HTMLAttributes } from "react"; | ||
|
|
||
| const Menu = DropdownMenu.Root; | ||
| const Trigger = DropdownMenu.Trigger; | ||
|
|
||
| interface IMenuContent | ||
| extends ComponentPropsWithoutRef<typeof DropdownMenu.Content> {} | ||
|
|
||
| const Content = ({ className, ...props }: IMenuContent) => ( | ||
| <DropdownMenu.Portal> | ||
| <DropdownMenu.Content | ||
| side="bottom" | ||
| align="start" | ||
| className={cn( | ||
| "bg-white border-2 border-black shadow-md absolute top-2 min-w-20", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| </DropdownMenu.Portal> | ||
| ); | ||
|
|
||
| const MenuItem = React.forwardRef< | ||
| HTMLDivElement, | ||
| ComponentPropsWithoutRef<typeof DropdownMenu.Item> | ||
| >(({ className, ...props }, ref) => ( | ||
| <DropdownMenu.Item | ||
| ref={ref} | ||
| className={cn( | ||
| "relative flex cursor-default select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none transition-colors hover:bg-primary-400 hover:text-white focus:bg-primary-400 focus:text-white data-[disabled]:pointer-events-none data-[disabled]:opacity-50", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| )); | ||
| MenuItem.displayName = "MenuItem"; | ||
|
|
||
| const MenuComponent = Object.assign(Menu, { | ||
| Trigger, | ||
| Content, | ||
| Item: MenuItem, | ||
| }); | ||
|
|
||
| export { MenuComponent as Menu }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from "./Menu"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider automating version management.
Instead of hardcoding the version, consider deriving it from the GitHub release tag to prevent version mismatches and reduce manual updates.
📝 Committable suggestion