Skip to content

feat: add examples dropdown also in code editor panel #50

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.

Sign up for GitHub

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 2 commits into from
Jul 31, 2025
Merged

feat: add examples dropdown also in code editor panel #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4fbdd78
fix: apply biome to src files
brettkolodny Jul 31, 2025
1d2cb87
src/client/editor/Editor.tsx
brettkolodny Jul 31, 2025
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
2 changes: 1 addition & 1 deletion biome.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"clientKind": "git"
},
"files": {
"includes": ["!src/client/gen/types.ts","!pnpm-lock.yaml"],
"includes": ["src/**/*", "!src/client/gen/types.ts","!pnpm-lock.yaml"],
"ignoreUnknown": true
},
"linter": {
Expand Down
86 changes: 61 additions & 25 deletions src/client/editor/Editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import {
CheckIcon,
ChevronDownIcon,
CopyIcon,
ExternalLinkIcon,
FileJsonIcon,
NotebookPenIcon,
PlusIcon,
UsersIcon,
ZapIcon,
} from "lucide-react";
Expand All @@ -22,6 +25,7 @@ import { useEditor } from "@/client/contexts/editor";
import { useTheme } from "@/client/contexts/theme";
import { Users } from "@/client/editor/Users";
import { type SnippetFunc, snippets } from "@/client/snippets";
import { examples } from "@/examples";
import type { ParameterWithSource } from "@/gen/types";
import type { User } from "@/user";
import { cn } from "@/utils/cn";
Expand Down Expand Up @@ -89,31 +93,63 @@ export const Editor: FC<EditorProps> = ({
<Tabs.Trigger icon={UsersIcon} label="Users" value="users" />
</div>

<DropdownMenu>
<DropdownMenuTrigger className="flex w-fit min-w-[140px] cursor-pointer items-center justify-between rounded-md border bg-surface-primary px-2 py-1.5 text-content-secondary transition-colors hover:text-content-primary data-[state=open]:text-content-primary">
<div className="flex items-center justify-center gap-2">
<ZapIcon width={18} height={18} />
<p className="text-xs">Snippets</p>
</div>
<ChevronDownIcon width={18} height={18} />
</DropdownMenuTrigger>

<DropdownMenuPortal>
<DropdownMenuContent align="start">
{snippets.map(
({ name, label, icon: Icon, snippet }, index) => (
<DropdownMenuItem
key={index}
onClick={() => onAddSnippet(name, snippet)}
>
<Icon size={24} />
{label}
</DropdownMenuItem>
),
)}
</DropdownMenuContent>
</DropdownMenuPortal>
</DropdownMenu>
<div className="flex items-center gap-2">
<DropdownMenu>
<DropdownMenuTrigger className="flex w-fit min-w-[140px] cursor-pointer items-center justify-between rounded-md border bg-surface-primary px-2 py-1.5 text-content-secondary transition-colors hover:text-content-primary data-[state=open]:text-content-primary">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to say that the default output for a Radix trigger is a button element, so putting two elements that are semantically block-based (div and p) inside it is going to make the HTML invalid

Two things come to mind:

  1. We might be able to switch the markup around. So:
    1. Get rid of the div and move the classes to the trigger
    2. Swap the p for a span
  2. I don't know how if there are any edge cases, but you might be able to use asChild to merge the trigger into first immediate child. But even then, you'd want to swap the div for a button

<div className="flex items-center justify-center gap-2">
<ZapIcon width={18} height={18} />
<p className="text-xs">Snippets</p>
</div>
<PlusIcon width={18} height={18} />
</DropdownMenuTrigger>

<DropdownMenuPortal>
<DropdownMenuContent align="start">
{snippets.map(
({ name, label, icon: Icon, snippet }, index) => (
<DropdownMenuItem
key={index}
Comment on lines +109 to +111
Copy link
Member

@Parkreiner Parkreiner Jul 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a guarantee that each snippet will be unique? In that case, I think we should get rid of the index, and use some combination of the name and label as the render key. If neither are guaranteed to be fully unique on their own, we could also concatenate them to make a compound key

onClick={() => onAddSnippet(name, snippet)}
>
<Icon size={24} />
{label}
</DropdownMenuItem>
),
)}
</DropdownMenuContent>
</DropdownMenuPortal>
</DropdownMenu>

<DropdownMenu>
<DropdownMenuTrigger className="flex w-fit min-w-[140px] cursor-pointer items-center justify-between rounded-md border bg-surface-primary px-2 py-1.5 text-content-secondary transition-colors hover:text-content-primary data-[state=open]:text-content-primary">
<div className="flex items-center justify-center gap-2">
<NotebookPenIcon width={18} height={18} />
<p className="text-xs">Examples</p>
</div>
<ChevronDownIcon width={18} height={18} />
</DropdownMenuTrigger>
Comment on lines +124 to +130
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same feedback for the trigger above applies here


<DropdownMenuPortal>
<DropdownMenuContent>
{Object.entries(examples).map(([slug, title]) => {
Copy link
Member

@Parkreiner Parkreiner Jul 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realistically this isn't a huge deal, but I think that even now, the JS spec doesn't guarantee that Object.entries makes an array in a deterministic order. The v8 engine does, but I don't know about other engines (I would expect them to follow v8's lead, though)

What this means is that without a .sort call, there's a risk that the elements could get scrambled up during re-renders. The stable keys will mean that the state won't get mixed up, but there's a risk the list item order will

const href = `${window.location.origin}/parameters/example/${slug}`;
return (
<DropdownMenuItem key={slug} asChild={true}>
<a href={href} target="_blank" rel="noreferrer">
<span className="sr-only">
{" "}
(link opens in new tab)
</span>
<ExternalLinkIcon />
{title}
</a>
</DropdownMenuItem>
);
})}
</DropdownMenuContent>
</DropdownMenuPortal>
</DropdownMenu>
</div>
</div>
</Tabs.List>

Expand Down