-
-
Notifications
You must be signed in to change notification settings - Fork 290
feat: add design review skill for claude #831
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
Closed
Closed
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
82db3b9
feat: add design review skill for claude
amaan-bhati 5a77b9b
feat: add repo specific skill and improve skill.md
amaan-bhati fc04a09
fix: fix paths, reference docs and align styling guidance
amaan-bhati 04024c3
fix: add when to review patch details in the files
amaan-bhati 1380215
Merge branch 'main' into design-review-skill
nehagup File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| name: design-agent | ||
| description: > | ||
| Use this skill when a contributor asks for /design-agent, design review, PR review, UI review, design feedback, visual QA, frontend consistency checks, component usage guidance, Tailwind review, CSS review, docs-site styling review, Docusaurus theme review, accessibility review, spacing review, typography review, or help matching Keploy Docs design conventions. Trigger this for PR review comments about design consistency, component usage, Tailwind/CSS conventions in this repo, hardcoded colors, inline styles, Docusaurus CSS variable usage, dark mode mismatches, focus states, and whether a page or component looks like the rest of the Keploy documentation site. | ||
|
|
||
|
amaan-bhati marked this conversation as resolved.
|
||
| Design Agent — Keploy Documentation | ||
| This agent reviews UI and frontend changes in the Keploy docs site, which is built on Docusaurus 3 with React, Tailwind CSS, Docusaurus theme overrides, and a large set of MDX helper components. It checks whether changes align with the repo's actual design system: Docusaurus CSS variables for core theming, Tailwind utility patterns on newer pages, and existing reusable components for chips, cards, quickstart flows, glossary cards, and docs metadata. | ||
|
|
||
| Trigger | ||
| This skill is invoked when a contributor uses /design-agent or asks for design review, design feedback, or UI/UX guidance on a PR or commit. | ||
|
|
||
| What to review in a PR | ||
| When called on a PR or set of code changes, check for: | ||
|
|
||
| 1. Token compliance | ||
| Colors must prefer the repo's existing theme tokens from `src/css/custom.css`: `--ifm-color-primary` `#ff914d`, `--ifm-color-primary-dark` `#e67643`, `--ifm-color-primary-darker` `#c95919`, `--ifm-color-primary-darkest` `#be2c1b`, `--ifm-color-primary-light` `#ffd0a0`, `--ifm-color-primary-lightest` `#ffceb1`, `--ifm-color` `#00163d` in light mode and `#f5f6f7` in dark mode, `--ifm-background-color` `rgb(249, 250, 251)` in light mode and `#141414` in dark mode, `--ifm-card-background-color`, and `--ifm-card-shadow-color`. | ||
| Use custom Tailwind colors from `tailwind.config.js` only when they already match an established pattern: `offwhite`, `keployblue`, `keploybrightblue`, `keploypurple`, `keploybrightpurple`, `spaceblack`, `green1`, `green2`, `orange1`, `orange2`, `gray5`, `lightgray`, `lightteal`. | ||
| Do not introduce arbitrary values such as `bg-[#3b82f6]`, `text-[#7c3aed]`, or new one-off RGB values when an existing Docusaurus variable or existing Tailwind palette class already fits. | ||
| `[inferred]` Newer homepage sections frequently use Tailwind's default gray/orange/purple/indigo families. Reuse those exact class families when extending those sections instead of inventing a new accent system. | ||
| Flag `style={{ color: "#..." }}`, `style={{ background: "#..." }}`, and ad hoc token maps with raw hex values unless the file is already following that legacy pattern and the PR cannot reasonably normalize it. | ||
|
|
||
| 2. Component usage | ||
| Use `@docusaurus/Link` for internal navigation, not raw `<a href="/docs/...">`. | ||
| Use `@theme/Heading` or markdown headings inside docs content so anchor links and scroll offsets keep working. | ||
| Use `GlossaryCard` for glossary card grids. | ||
| Use `DocHeaderChips`, `ProductTier`, or `TierCallout` for doc metadata chips instead of inventing another badge row. | ||
| Use `QuickStartTabs` and `QuickStartFilter` when editing the quickstart entry experience; do not rebuild the same chooser pattern from scratch. | ||
| Use `SidebarBadge` and `SidebarCategoryIcon` for sidebar labels/icons instead of ad hoc badge markup. | ||
| `[inferred]` Reuse existing card anatomy from newer homepage sections: `rounded-2xl`, bordered surfaces, `p-6`, compact eyebrow pill, `text-3xl md:text-4xl` section headings, and dark-mode variants. | ||
| There is no consistently enforced global button component in this repo. Do not force `src/components/shared/Button.js` everywhere, but do flag raw buttons/links that ignore existing CTA patterns, focus states, or dark-mode treatment. | ||
|
|
||
| 3. Typography | ||
| Docs pages force `DM Sans` globally through `src/css/custom.css`; code blocks intentionally revert to the system monospace stack. | ||
| Headings and the navbar use an `Aeonik`-first fallback stack in `src/css/custom.css`; body copy on docs pages is still forced to `DM Sans`. | ||
| Review heading hierarchy carefully: section headers on newer landing sections usually use `text-3xl font-bold tracking-tight md:text-4xl`; subheads are commonly `text-lg` or `text-xl` with `font-bold` or `font-semibold`; supporting copy is usually `text-sm`, `text-base`, or `text-xs` in gray text classes. | ||
| In docs markdown, prose styling comes from Tailwind Typography plus Docusaurus overrides in `tailwind.config.js` and `src/theme/DocItem/index.js`. | ||
| Flag jumps in hierarchy like using `text-4xl` for small card titles or mixing legacy `tracking-wide font-semibold` headings with the newer `tracking-tight font-bold` marketing pattern in the same section. | ||
|
|
||
| 4. Spacing & layout | ||
| The homepage shell uses `max-w-screen-lg` with `p-6 md:p-10` and stacked sections separated mostly by `mb-12` or `mb-16`. | ||
| Common card spacing is `p-6`; compact nested options often use `p-3` or `p-4`. | ||
| Common gaps are `gap-2`, `gap-3`, `gap-4`, and `gap-6`. | ||
| Common max widths are `max-w-2xl`, `max-w-3xl`, `max-w-5xl`, and `max-w-6xl`. | ||
| `[inferred]` Prefer `rounded-xl` or `rounded-2xl` on newer surfaces; `rounded-lg` appears mostly in older components. | ||
| Flag new layouts that ignore the established page width, use inconsistent spacing steps, or introduce oversized padding/margins without a clear reason. | ||
|
|
||
| 5. Responsiveness | ||
| This repo uses Tailwind's default responsive prefixes; no custom breakpoints are defined in `tailwind.config.js`. | ||
| Common responsive patterns in source are `md:grid-cols-2`, `md:grid-cols-3`, `md:grid-cols-12`, `md:text-4xl`, `md:p-10`, `lg:gap-8`, `lg:px-10`, and Docusaurus' desktop breakpoint behavior around `997px` in `src/theme/DocItem/styles.module.css`. | ||
| `[inferred]` New sections should collapse to one column on mobile, keep CTAs full-width or easy to tap, and avoid text that depends on hover-only disclosure. | ||
| Flag any PR that adds fixed widths or desktop-only multi-column layouts without a mobile fallback. | ||
|
|
||
| 6. Accessibility | ||
| Keep visible focus treatment. Global focus styles are defined in `src/css/custom.css`, and some components add stronger local rings with `focus-visible:ring-*`. | ||
| Preserve `aria-label` usage for icon-only links and action buttons; examples already exist in `src/theme/DocItem/index.js`, `src/theme/Heading/index.js`, `src/pages/concepts/reference/glossary.js`, and `src/components/WhatIsKeploy.js`. | ||
| Images should keep meaningful `alt` text unless decorative. | ||
| Internal controls that behave like toggles or filters should keep state semantics such as `aria-pressed`. | ||
| If a PR adds `target="_blank"`, require `rel="noopener noreferrer"`. | ||
| Flag `focus:outline-none` when it is not paired with an equivalent replacement ring or outline. | ||
|
|
||
| 7. Anti-patterns to flag | ||
| Inline styling for structure, spacing, or color in React components, especially in `InstallReminder`, `EnterpriseInstallReminder`, `SectionDivider`, `StartKeploy`, `StartKeployDocker`, and `ResponsivePlayer`. | ||
| Hardcoded color maps and local `<style>` blocks for reusable chips/badges instead of shared tokens or CSS variables, as seen in `DocHeaderChips`, `SidebarBadge`, `ProductTier`, and `TierCallout`. | ||
| Arbitrary value classes like `bg-[color:orange]`, `border-[color:orange]`, `bg-[var(--ifm-card-background-color)]`, and `shadow-[0_4px_12px_var(--ifm-card-shadow-color)]` when a standard class or central token would be clearer. | ||
| Invalid or inconsistent JSX/SVG attributes such as raw `class=` and kebab-case `fill-rule` or `clip-rule`. | ||
| Recreating cards, pill badges, or quickstart selectors that already exist in `src/components`. | ||
| Mixing old `rounded-lg shadow-lg` legacy cards into newer `rounded-2xl border p-6` sections without an intentional visual reason. | ||
| Introducing new font families or typography systems; this repo already has conflicting font choices and should not add more. | ||
|
|
||
| How to deliver feedback | ||
| When reviewing a PR, structure your feedback as follows: | ||
|
|
||
| Summary — one paragraph overall design health assessment | ||
| Critical issues — things that must be fixed before merge (broken design tokens, accessibility failures, missing required components) | ||
| Suggestions — things that would improve consistency but aren't blockers | ||
| Positive notes — what was done well (always include at least one) | ||
|
|
||
| Be specific. Reference actual line numbers or file names from the diff. | ||
| Compare against guidelines in the references/ files. | ||
| Never give vague feedback like "improve spacing" — say exactly what class or token should be used instead. | ||
|
|
||
| Reference files | ||
|
|
||
| Read references/design-tokens.md for all color, spacing, and typography values | ||
| Read references/component-library.md for component usage rules | ||
| Read references/anti-patterns.md for patterns to flag in review | ||
299 changes: 299 additions & 0 deletions
299
.claude/skills/design-agent/references/anti-patterns.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,299 @@ | ||
| # Keploy Docs anti-patterns | ||
|
|
||
| These are patterns already present in the repo that a design-review agent should flag in PRs instead of normalizing as "okay". | ||
|
|
||
| ## 1. Inline styles for color, spacing, borders, or layout | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| <div | ||
| style={{ | ||
| padding: "1rem", | ||
| border: isDark ? "1px solid #333" : "1px solid #eee", | ||
| borderRadius: "10px", | ||
| background: isDark ? "#23272f" : "#fff8f5", | ||
| }} | ||
| /> | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - It bypasses both Docusaurus theme variables and Tailwind utility conventions. | ||
| - It makes dark mode, token reuse, and visual consistency harder to review. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| <div className="rounded-xl border border-gray-200 bg-[var(--ifm-card-background-color)] p-4 dark:border-gray-700" /> | ||
| ``` | ||
|
|
||
| Seen in: | ||
|
|
||
| - `src/components/InstallReminder.js` | ||
| - `src/components/EnterpriseInstallReminder.js` | ||
| - `src/components/SectionDivider.js` | ||
| - `src/components/StartKeploy.js` | ||
| - `src/components/StartKeployDocker.js` | ||
|
|
||
| ## 2. Hardcoded hex colors instead of repo tokens | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| const chipStyles = { | ||
| enterprise: { label: "Enterprise", color: "#7c3aed", bg: "rgba(139, 92, 246, 0.1)" }, | ||
| cloud: { label: "Cloud", color: "#2563eb", bg: "rgba(59, 130, 246, 0.1)" }, | ||
| }; | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - The repo already has Docusaurus CSS variables and a Tailwind palette. | ||
| - Repeating hardcoded hex values in component-local maps creates token drift. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| <span className="rounded-full bg-purple-100 px-2 py-1 text-purple-700 dark:bg-purple-900/30 dark:text-purple-200" /> | ||
| ``` | ||
|
|
||
| Or, for doc-wide theming: | ||
|
|
||
| ```jsx | ||
| <span style={{ color: "var(--ifm-color-primary)" }} /> | ||
|
amaan-bhati marked this conversation as resolved.
Outdated
|
||
| ``` | ||
|
|
||
| Seen in: | ||
|
|
||
| - `src/components/DocHeaderChips.js` | ||
| - `src/components/SidebarBadge.js` | ||
| - `src/components/ProductTier.js` | ||
| - `src/components/TierCallout.js` | ||
| - `src/components/SidebarCategoryIcon.js` | ||
|
|
||
| ## 3. Arbitrary-value classes where standard classes or shared tokens would be clearer | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| className="bg-[color:orange] border-[color:orange] bg-[var(--ifm-card-background-color)] shadow-[0_4px_12px_var(--ifm-card-shadow-color)]" | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - Arbitrary values make review harder because they hide the intended token or design choice. | ||
| - Some are especially brittle or unclear, such as `bg-[color:orange]`. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| className="bg-orange-500 border-orange-500" | ||
| ``` | ||
|
|
||
| Or: | ||
|
|
||
| ```jsx | ||
| className="bg-[var(--ifm-card-background-color)]" | ||
| ``` | ||
|
|
||
| only when there is no standard class and the Docusaurus variable is intentional. | ||
|
|
||
| Seen in: | ||
|
|
||
| - `src/components/GSoC.js` | ||
| - `src/components/Hacktoberfest.js` | ||
| - `src/components/GitTogether.js` | ||
| - `src/components/GlossaryCard.js` | ||
| - `src/components/QuickStart.js` | ||
|
|
||
| ## 4. Invalid JSX or SVG attribute names | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| <div class="flex gap-3"> | ||
| ``` | ||
|
|
||
| ```jsx | ||
| <path fill-rule="evenodd" clip-rule="evenodd" /> | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - `class` should be `className` in JSX. | ||
| - SVG attributes must use camelCase in React: `fillRule`, `clipRule`. | ||
| - These issues are easy to miss visually but create avoidable warnings and inconsistency. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| <div className="flex gap-3"> | ||
| ``` | ||
|
|
||
| ```jsx | ||
| <path fillRule="evenodd" clipRule="evenodd" /> | ||
| ``` | ||
|
|
||
| Seen in: | ||
|
|
||
| - `src/components/QuickStart.js` | ||
| - `src/components/Product.js` | ||
|
|
||
| ## 5. Mixing multiple styling systems inside one component without a clear reason | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| <div className="quickstart-wizard"> | ||
| <style>{`...large CSS block...`}</style> | ||
| <div className="wizard-option" style={{ flexDirection: "column" }} /> | ||
| </div> | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - It combines Tailwind, large inline `<style>` blocks, and per-node inline styles. | ||
| - That makes future edits slower and makes consistency review harder. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| - Prefer one primary system per component. | ||
| - In this repo, newer components are easiest to maintain when they stay Tailwind-first and only use CSS variables where theming requires it. | ||
|
|
||
| Seen in: | ||
|
|
||
| - `src/components/QuickStartFilter.js` | ||
|
|
||
| ## 6. Recreating badge, chip, and metadata systems instead of reusing the existing helpers | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| <span className="rounded-full bg-orange-100 px-3 py-1 text-sm font-medium text-orange-600"> | ||
| Custom status | ||
| </span> | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - This repo already has several metadata helpers: `DocHeaderChips`, `ProductTier`, `TierCallout`, and `SidebarBadge`. | ||
| - Recreating them increases visual drift. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| <TierCallout chips={["oss", "docker"]} /> | ||
| ``` | ||
|
|
||
| or | ||
|
|
||
| ```jsx | ||
| <DocHeaderChips tier="cloud" version="4.0.0" /> | ||
| ``` | ||
|
|
||
| Seen in: | ||
|
|
||
| - Repeated manually across several homepage sections | ||
|
|
||
| ## 7. Introducing more legacy card styles into new sections | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| <div className="rounded-lg bg-[color:var(--ifm-card-background-color)] p-5 shadow-lg" /> | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - The repo already has a visible split between older card styling and newer homepage styling. | ||
| - New work should follow the newer pattern unless it is intentionally editing a legacy section in place. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| <div className="rounded-2xl border border-gray-200 bg-white p-6 shadow-sm dark:border-gray-700 dark:bg-gray-800/50" /> | ||
| ``` | ||
|
|
||
| Seen in: | ||
|
|
||
| - `src/components/QuickStart.js` | ||
| - `src/components/SDKs.js` | ||
| - `src/components/Intro.js` | ||
| - `src/components/Product.js` | ||
| - `src/components/UtgMethods.js` | ||
|
|
||
| ## 8. `focus:outline-none` without a clear replacement | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| className="focus:outline-none" | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - The repo already defines global focus styling and some components add stronger `focus-visible` rings. | ||
| - Removing outlines without a replacement harms keyboard usability. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| className="focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[var(--ifm-color-primary)] focus-visible:ring-offset-2" | ||
| ``` | ||
|
|
||
| Seen in: | ||
|
|
||
| - Any new PR that removes focus styling | ||
| - Existing correct replacement examples: `src/components/WhatIsKeploy.js`, `src/components/GlossaryCard.js`, `src/pages/concepts/reference/glossary.js`, `src/components/shared/Button.js` | ||
|
|
||
| ## 9. Duplicated IDs inside reusable components | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```jsx | ||
| <button id="copy-full-code">Copy</button> | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - Reusable components may render multiple times on one page. | ||
| - Duplicate IDs break DOM assumptions and make styling or scripting unreliable. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| ```jsx | ||
| <button onClick={handleCopy}>Copy</button> | ||
| ``` | ||
|
|
||
| Or generate a unique ID per instance. | ||
|
|
||
| Seen in: | ||
|
|
||
| - `src/components/CollapsibleCode.js` | ||
|
|
||
| ## 10. Adding more one-off font systems | ||
|
|
||
| What it looks like: | ||
|
|
||
| ```css | ||
| h1, | ||
| h2, | ||
| h3 { | ||
| font-family: "SomeNewBrandFont", sans-serif; | ||
| } | ||
| ``` | ||
|
|
||
| Why it's wrong: | ||
|
|
||
| - The repo already mixes `DM Sans`, `Aeonik`, and local `Roboto`. | ||
| - Adding another font family worsens inconsistency. | ||
|
|
||
| Correct alternative: | ||
|
|
||
| - Reuse the existing docs body and heading stacks. | ||
| - If typography needs cleanup, centralize it in `src/css/custom.css` instead of per-component overrides. | ||
|
|
||
| Seen in: | ||
|
|
||
| - Existing font split between docs body and headings in `src/css/custom.css` | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.