Skip to content
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

Jump to bottom

feat: design system #2667

Merged
merged 4 commits into from
Nov 28, 2024
Merged

feat: design system #2667

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2825c03
feat: design system
chronark Nov 18, 2024
e94dab7
feat: button
chronark Nov 28, 2024
09f8163
feat: button
chronark Nov 28, 2024
37be42c
feat: button
chronark Nov 28, 2024
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
19 changes: 19 additions & 0 deletions apps/engineering/app/components/render.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { CodeBlock, Pre } from "fumadocs-ui/components/codeblock";
import type { PropsWithChildren } from "react";
import reactElementToJSXString from "react-element-to-jsx-string";

export const RenderComponentWithSnippet: React.FC<PropsWithChildren> = (props) => {
return (
<div>
{props.children}
<CodeBlock>
<Pre>
{reactElementToJSXString(props.children, {
showFunctions: true,
useBooleanShorthandSyntax: true,
})}
</Pre>
</CodeBlock>
</div>
);
};
Comment on lines +5 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and improve type safety.

While the implementation is functional, consider these improvements for robustness:

  1. Add error boundaries to handle potential JSX conversion failures
  2. Add explicit return type annotation
  3. Consider using a more semantic wrapper with proper styling

Here's a suggested implementation:

-export const RenderComponentWithSnippet: React.FC<PropsWithChildren> = (props) => {
+export const RenderComponentWithSnippet: React.FC<PropsWithChildren> = (props): JSX.Element => {
+  const jsxString = React.useMemo(
+    () =>
+      reactElementToJSXString(props.children, {
+        showFunctions: true,
+        useBooleanShorthandSyntax: true,
+      }),
+    [props.children]
+  );
+
   return (
-    <div>
+    <section className="space-y-4">
       {props.children}
-      <CodeBlock>
-        <Pre>
-          {reactElementToJSXString(props.children, {
-            showFunctions: true,
-            useBooleanShorthandSyntax: true,
-          })}
-        </Pre>
-      </CodeBlock>
-    </div>
+      <ErrorBoundary fallback={<div>Failed to generate code snippet</div>}>
+        <CodeBlock>
+          <Pre>{jsxString}</Pre>
+        </CodeBlock>
+      </ErrorBoundary>
+    </section>
   );
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const RenderComponentWithSnippet: React.FC<PropsWithChildren> = (props) => {
return (
<div>
{props.children}
<CodeBlock>
<Pre>
{reactElementToJSXString(props.children, {
showFunctions: true,
useBooleanShorthandSyntax: true,
})}
</Pre>
</CodeBlock>
</div>
);
};
export const RenderComponentWithSnippet: React.FC<PropsWithChildren> = (props): JSX.Element => {
const jsxString = React.useMemo(
() =>
reactElementToJSXString(props.children, {
showFunctions: true,
useBooleanShorthandSyntax: true,
}),
[props.children]
);
return (
<section className="space-y-4">
{props.children}
<ErrorBoundary fallback={<div>Failed to generate code snippet</div>}>
<CodeBlock>
<Pre>{jsxString}</Pre>
</CodeBlock>
</ErrorBoundary>
</section>
);
};

5 changes: 5 additions & 0 deletions apps/engineering/app/components/row.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { PropsWithChildren } from "react";

export const Row: React.FC<PropsWithChildren> = (props) => {
return <div className="flex w-full items-center justify-around gap-8">{props.children}</div>;
};
50 changes: 50 additions & 0 deletions apps/engineering/app/design/[[...slug]]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"use client";
import { componentSource } from "@/app/source";
import defaultMdxComponents from "fumadocs-ui/mdx";
import { DocsBody, DocsDescription, DocsPage, DocsTitle } from "fumadocs-ui/page";
//import { createTypeTable } from 'fumadocs-typescript/ui';

import { notFound } from "next/navigation";
export default async function Page(props: {
params: Promise<{ slug?: string[] }>;
}) {
Comment on lines +8 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider simplifying the props type

The params prop is wrapped in a Promise, which is unusual for Next.js page components. Typically, route parameters are directly available as plain objects.

Consider simplifying the type to:

-export default async function Page(props: {
-  params: Promise<{ slug?: string[] }>;
-}) {
+export default async function Page(props: {
+  params: { slug?: string[] };
+}) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default async function Page(props: {
params: Promise<{ slug?: string[] }>;
}) {
export default async function Page(props: {
params: { slug?: string[] };
}) {

const params = await props.params;

const page = componentSource.getPage(params.slug);

if (!page) {
notFound();
}
//const { AutoTypeTable } = createTypeTable();

const MDX = page.data.body;

return (
<DocsPage toc={page.data.toc} full={page.data.full}>
<DocsTitle>{page.data.title}</DocsTitle>

<DocsDescription>{page.data.description}</DocsDescription>

<DocsBody>
<MDX components={{ ...defaultMdxComponents }} />
</DocsBody>
</DocsPage>
);
}
/*
export async function generateStaticParams() {
return componentSource.generateParams();
}

export function generateMetadata({ params }: { params: { slug?: string[] } }) {
const page = componentSource.getPage(params.slug);
if (!page) {
notFound();
}

return {
title: page.data.title,
description: page.data.description,
} satisfies Metadata;
}
*/
Comment on lines +34 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Based on the output, I can now provide a final response:

Enable metadata and static generation functions

The commented-out functions should be uncommented because:

  1. Other pages in the engineering app (docs/[[...slug]]/page.tsx and rfcs/[[...slug]]/page.tsx) use these functions for proper metadata and static generation
  2. These functions are using the same componentSource pattern as the other pages
  3. The implementation matches the pattern used in other pages

Please uncomment these functions to ensure consistent behavior with other pages in the engineering app, particularly for:

  • Static site generation optimization through generateStaticParams
  • SEO and page metadata through generateMetadata
🔗 Analysis chain

Enable metadata and static generation functions

The commented-out generateStaticParams and generateMetadata functions are important for:

  1. SEO optimization through proper metadata
  2. Static site generation for better performance
  3. Type-safe metadata generation

Consider uncommenting and enabling these functions unless there's a specific reason for disabling them.

Let's check if other pages implement these functions:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for generateMetadata and generateStaticParams usage in other pages
rg "export (async )?function generate(Metadata|StaticParams)" --type tsx

Length of output: 102

Script:

#!/bin/bash
# Check for generateMetadata and generateStaticParams usage in other pages with proper file extension
rg "export (async )?function generate(Metadata|StaticParams)" -t ts -t tsx

# Let's also check if there are any other page.tsx files
fd "page.tsx$"

# And specifically look at their content for metadata-related code
fd "page.tsx$" -x cat {} \;

Length of output: 114233

Script:

#!/bin/bash
# Let's check if generateMetadata and generateStaticParams are used in other pages in the engineering app
fd "page.tsx$" apps/engineering/ -x cat {} \;

# Also check if there are any imports of Metadata type from next
rg "import.*Metadata.*from.*next" -t ts -t tsx apps/engineering/

Length of output: 5666

12 changes: 12 additions & 0 deletions apps/engineering/app/design/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { componentSource } from "@/app/source";
import "@unkey/ui/css";
import { DocsLayout } from "fumadocs-ui/layouts/notebook";
import type { ReactNode } from "react";
import { baseOptions } from "../layout.config";
export default function Layout({ children }: { children: ReactNode }) {
return (
<DocsLayout tree={componentSource.pageTree} {...baseOptions}>
{children}
</DocsLayout>
);
}
13 changes: 6 additions & 7 deletions apps/engineering/app/docs/[[...slug]]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import { DocsBody, DocsDescription, DocsPage, DocsTitle } from "fumadocs-ui/page
import type { Metadata } from "next";

import { notFound } from "next/navigation";

export default async function Page({
params,
}: {
params: { slug?: string[] };
export default async function Page(props: {
params: Promise<{ slug?: string[] }>;
}) {
const params = await props.params;

const page = source.getPage(params.slug);
if (!page) {
notFound();
Expand Down Expand Up @@ -53,8 +52,8 @@ export async function generateStaticParams() {
return source.generateParams();
}

export function generateMetadata({ params }: { params: { slug?: string[] } }) {
const page = source.getPage(params.slug);
export async function generateMetadata({ params }: { params: Promise<{ slug?: string[] }> }) {
const page = source.getPage((await params).slug);
if (!page) {
notFound();
}
Expand Down
5 changes: 5 additions & 0 deletions apps/engineering/app/layout.config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ export const baseOptions: HomeLayoutProps = {
url: "/rfcs",
active: "nested-url",
},
{
text: "Design",
url: "/design",
active: "nested-url",
},
{
text: "GitHub",
url: "https://github.com/unkeyed/unkey",
Expand Down
15 changes: 9 additions & 6 deletions apps/engineering/app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import { RootProvider } from "fumadocs-ui/provider";
import { Inter } from "next/font/google";
import { GeistMono } from "geist/font/mono";
import { GeistSans } from "geist/font/sans";

import type { ReactNode } from "react";
import "./global.css";

const inter = Inter({
subsets: ["latin"],
});
import "./global.css";

export default function Layout({ children }: { children: ReactNode }) {
return (
<html lang="en" className={inter.className} suppressHydrationWarning>
<html
lang="en"
className={`${GeistSans.variable} ${GeistMono.variable}`}
suppressHydrationWarning
>
<body>
<RootProvider>{children}</RootProvider>
</body>
Expand Down
12 changes: 6 additions & 6 deletions apps/engineering/app/rfcs/[[...slug]]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { LocalDate } from "./local-date";

import { notFound } from "next/navigation";

export default async function Page({
params,
}: {
params: { slug?: string[] };
export default async function Page(props: {
params: Promise<{ slug?: string[] }>;
}) {
const params = await props.params;

const page = rfcSource.getPage(params.slug);

if (!page) {
Expand Down Expand Up @@ -60,8 +60,8 @@ export async function generateStaticParams() {
return rfcSource.generateParams();
}

export function generateMetadata({ params }: { params: { slug?: string[] } }) {
const page = rfcSource.getPage(params.slug);
export async function generateMetadata({ params }: { params: Promise<{ slug?: string[] }> }) {
const page = rfcSource.getPage((await params).slug);
if (!page) {
notFound();
}
Expand Down
7 changes: 6 additions & 1 deletion apps/engineering/app/source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { docs, meta, rfcs } from "@/.source";
import { components, docs, meta, rfcs } from "@/.source";
import { loader } from "fumadocs-core/source";
import { createMDXSource } from "fumadocs-mdx";

Expand All @@ -11,3 +11,8 @@ export const rfcSource = loader({
baseUrl: "/rfcs",
source: createMDXSource(rfcs, []),
});

export const componentSource = loader({
baseUrl: "/design",
source: createMDXSource(components, []),
});
142 changes: 142 additions & 0 deletions apps/engineering/content/design/colors.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
---
title: Colors
description: Color scale and usage
---







Unkey uses [Radix Colors](https://www.radix-ui.com/colors) and its scale.

| Step | Use Case |
|-------|-----------------------------------------|
| 1 | App background |
| 2 | Subtle background |
| 3 | UI element background |
| 4 | Hovered UI element background |
| 5 | Active / Selected UI element background |
| 6 | Subtle borders and separators |
| 7 | UI element border and focus rings |
| 8 | Hovered UI element border |
| 9 | Solid backgrounds |
| 10 | Hovered solid backgrounds |
| 11 | Low-contrast text |
| 12 | High-contrast text |

[Read more](https://www.radix-ui.com/colors/docs/palette-composition/understanding-the-scale)


## gray
<div className="grid grid-cols-12 gap-2">
<div className="rounded-lg aspect-square bg-gray-1"/>
<div className="rounded-lg aspect-square bg-gray-2"/>
<div className="rounded-lg aspect-square bg-gray-3"/>
<div className="rounded-lg aspect-square bg-gray-4"/>
<div className="rounded-lg aspect-square bg-gray-5"/>
<div className="rounded-lg aspect-square bg-gray-6"/>
<div className="rounded-lg aspect-square bg-gray-7"/>
<div className="rounded-lg aspect-square bg-gray-8"/>
<div className="rounded-lg aspect-square bg-gray-9"/>
<div className="rounded-lg aspect-square bg-gray-10"/>
<div className="rounded-lg aspect-square bg-gray-11"/>
<div className="rounded-lg aspect-square bg-gray-12"/>
</div>
Comment on lines +34 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance color grid accessibility and maintainability

The color grids could benefit from several improvements:

  1. Add ARIA labels for screen readers
  2. Display color values (hex/rgb) on hover
  3. Consider extracting the repeated grid structure into a reusable component

Here's an example of how to improve one color grid:

-<div className="grid grid-cols-12 gap-2">
-  <div className="rounded-lg aspect-square bg-gray-1"/>
-  <div className="rounded-lg aspect-square bg-gray-2"/>
+<div className="grid grid-cols-12 gap-2" role="group" aria-label="Gray color scale">
+  <div 
+    className="rounded-lg aspect-square bg-gray-1 group relative"
+    aria-label="Gray shade 1"
+  >
+    <span className="opacity-0 group-hover:opacity-100 absolute bottom-0 left-0 right-0 text-xs p-1 bg-black/50 text-white text-center">
+      Gray 1
+    </span>
+  </div>
+  <div 
+    className="rounded-lg aspect-square bg-gray-2 group relative"
+    aria-label="Gray shade 2"
+  >
+    <span className="opacity-0 group-hover:opacity-100 absolute bottom-0 left-0 right-0 text-xs p-1 bg-black/50 text-white text-center">
+      Gray 2
+    </span>
+  </div>

Consider creating a reusable component to reduce repetition:

type ColorSwatchProps = {
  colorName: string;
  shades: number[];
};

function ColorSwatchGrid({ colorName, shades }: ColorSwatchProps) {
  return (
    <div className="grid grid-cols-12 gap-2" role="group" aria-label={`${colorName} color scale`}>
      {shades.map((shade) => (
        <div
          key={shade}
          className={`rounded-lg aspect-square bg-${colorName}-${shade} group relative`}
          aria-label={`${colorName} shade ${shade}`}
        >
          <span className="opacity-0 group-hover:opacity-100 absolute bottom-0 left-0 right-0 text-xs p-1 bg-black/50 text-white text-center">
            {`${colorName} ${shade}`}
          </span>
        </div>
      ))}
    </div>
  );
}

Also applies to: 49-62, 65-78, 96-110, 112-126, 128-142


## success
<div className="grid grid-cols-12 gap-2">
<div className="rounded-lg aspect-square bg-success-1"/>
<div className="rounded-lg aspect-square bg-success-2"/>
<div className="rounded-lg aspect-square bg-success-3"/>
<div className="rounded-lg aspect-square bg-success-4"/>
<div className="rounded-lg aspect-square bg-success-5"/>
<div className="rounded-lg aspect-square bg-success-6"/>
<div className="rounded-lg aspect-square bg-success-7"/>
<div className="rounded-lg aspect-square bg-success-8"/>
<div className="rounded-lg aspect-square bg-success-9"/>
<div className="rounded-lg aspect-square bg-success-10"/>
<div className="rounded-lg aspect-square bg-success-11"/>
<div className="rounded-lg aspect-square bg-success-12"/>
</div>

## info
<div className="grid grid-cols-12 gap-2">
<div className="rounded-lg aspect-square bg-info-1"/>
<div className="rounded-lg aspect-square bg-info-2"/>
<div className="rounded-lg aspect-square bg-info-3"/>
<div className="rounded-lg aspect-square bg-info-4"/>
<div className="rounded-lg aspect-square bg-info-5"/>
<div className="rounded-lg aspect-square bg-info-6"/>
<div className="rounded-lg aspect-square bg-info-7"/>
<div className="rounded-lg aspect-square bg-info-8"/>
<div className="rounded-lg aspect-square bg-info-9"/>
<div className="rounded-lg aspect-square bg-info-10"/>
<div className="rounded-lg aspect-square bg-info-11"/>
<div className="rounded-lg aspect-square bg-info-12"/>
</div>

## info
<div className="grid grid-cols-12 gap-2">
<div className="rounded-lg aspect-square bg-info-1"/>
<div className="rounded-lg aspect-square bg-info-2"/>
<div className="rounded-lg aspect-square bg-info-3"/>
<div className="rounded-lg aspect-square bg-info-4"/>
<div className="rounded-lg aspect-square bg-info-5"/>
<div className="rounded-lg aspect-square bg-info-6"/>
<div className="rounded-lg aspect-square bg-info-7"/>
<div className="rounded-lg aspect-square bg-info-8"/>
<div className="rounded-lg aspect-square bg-info-9"/>
<div className="rounded-lg aspect-square bg-info-10"/>
<div className="rounded-lg aspect-square bg-info-11"/>
<div className="rounded-lg aspect-square bg-info-12"/>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate "info" section

The "info" color grid is duplicated unnecessarily.

Remove the second occurrence of the info section (lines 80-94).


## warning
<div className="grid grid-cols-12 gap-2">
<div className="rounded-lg aspect-square bg-warning-1"/>
<div className="rounded-lg aspect-square bg-warning-2"/>
<div className="rounded-lg aspect-square bg-warning-3"/>
<div className="rounded-lg aspect-square bg-warning-4"/>
<div className="rounded-lg aspect-square bg-warning-5"/>
<div className="rounded-lg aspect-square bg-warning-6"/>
<div className="rounded-lg aspect-square bg-warning-7"/>
<div className="rounded-lg aspect-square bg-warning-8"/>
<div className="rounded-lg aspect-square bg-warning-9"/>
<div className="rounded-lg aspect-square bg-warning-10"/>
<div className="rounded-lg aspect-square bg-warning-11"/>
<div className="rounded-lg aspect-square bg-warning-12"/>
</div>

## error
<div className="grid grid-cols-12 gap-2">
<div className="rounded-lg aspect-square bg-error-1"/>
<div className="rounded-lg aspect-square bg-error-2"/>
<div className="rounded-lg aspect-square bg-error-3"/>
<div className="rounded-lg aspect-square bg-error-4"/>
<div className="rounded-lg aspect-square bg-error-5"/>
<div className="rounded-lg aspect-square bg-error-6"/>
<div className="rounded-lg aspect-square bg-error-7"/>
<div className="rounded-lg aspect-square bg-error-8"/>
<div className="rounded-lg aspect-square bg-error-9"/>
<div className="rounded-lg aspect-square bg-error-10"/>
<div className="rounded-lg aspect-square bg-error-11"/>
<div className="rounded-lg aspect-square bg-error-12"/>
</div>

## feature
<div className="grid grid-cols-12 gap-2">
<div className="rounded-lg aspect-square bg-feature-1"/>
<div className="rounded-lg aspect-square bg-feature-2"/>
<div className="rounded-lg aspect-square bg-feature-3"/>
<div className="rounded-lg aspect-square bg-feature-4"/>
<div className="rounded-lg aspect-square bg-feature-5"/>
<div className="rounded-lg aspect-square bg-feature-6"/>
<div className="rounded-lg aspect-square bg-feature-7"/>
<div className="rounded-lg aspect-square bg-feature-8"/>
<div className="rounded-lg aspect-square bg-feature-9"/>
<div className="rounded-lg aspect-square bg-feature-10"/>
<div className="rounded-lg aspect-square bg-feature-11"/>
<div className="rounded-lg aspect-square bg-feature-12"/>
</div>
Loading