-
-
Notifications
You must be signed in to change notification settings - Fork 289
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 all commits
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
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,57 @@ | ||
| name: design-agent | ||
| description: > | ||
| Use this skill when a contributor asks for /design-agent, docs design review, PR design feedback, Docusaurus UI review, docs-page consistency review, MDX component review, docs theme review, docs accessibility review, or help checking whether a change matches keploy/docs conventions beyond the shared Keploy design commons. Trigger it for reviews of docs-specific layout, Docusaurus theme overrides, sidebar/navigation behavior, docs metadata chips, prose styling, global docs CSS, and repo-local Tailwind/CSS patterns that are unique to keploy/docs. | ||
|
|
||
|
amaan-bhati marked this conversation as resolved.
amaan-bhati marked this conversation as resolved.
|
||
| Design Agent — Keploy Docs overrides | ||
| This skill reviews only the design-system rules that are specific to the `keploy/docs` repository. It assumes the shared Keploy commons are already loaded, then applies the Docusaurus-specific, docs-shell-specific, and repo-local component/style overrides found in this repository. | ||
|
|
||
| Loading order | ||
| Load commons guidelines first, then apply these repo-specific overrides. In case of conflict, repo-specific rules take precedence. | ||
|
|
||
| When to use | ||
| Use this skill for PRs or commits in `keploy/docs` that touch Docusaurus pages, MDX helpers, docs theme overrides, sidebar or breadcrumb behavior, docs layout, `src/css/custom.css`, `tailwind.config.js`, or any reusable component under `src/components` or `src/theme`. | ||
| Evaluate the current file contents in the PR diff before repeating feedback from earlier review rounds; do not assume an older comment still applies if the relevant text or implementation has already changed. | ||
|
|
||
| What to review | ||
| Review only docs-specific overrides beyond the commons: | ||
|
|
||
| 1. Docusaurus-specific tokens and theme variables | ||
| Check whether changes use the repo’s Docusaurus token layer in `src/css/custom.css` when editing docs surfaces, markdown content, inline code, cards, navbar, footer, or docs background treatment. | ||
| Flag changes that bypass docs-specific variables like `--ifm-color-primary`, `--ifm-color`, `--ifm-background-color`, `--ifm-card-background-color`, and `--ifm-card-shadow-color` when the change is clearly part of the docs shell rather than a one-off marketing section. | ||
|
|
||
| 2. Docs-shell components and theme overrides | ||
| Check whether PRs preserve the behavior of `src/theme/DocItem`, `src/theme/Heading`, `src/theme/DocBreadcrumbs`, and `src/theme/DocSidebarItem/Category`. | ||
| Flag raw replacements that break heading anchors, TOC behavior, breadcrumb semantics, sidebar icon treatment, or doc-page footer behavior. | ||
|
|
||
| 3. Docs-specific reusable components | ||
| Prefer the existing docs metadata and helper components when they match the job: `DocHeaderChips`, `ProductTier`, `TierCallout`, `SidebarBadge`, `SidebarCategoryIcon`, `GlossaryCard`, `QuickStartFilter`, `QuickStartTabs`, `StartKeploy`, `StartKeployDocker`, and `CollapsibleCode`. | ||
| Do not require these components outside their scope, but do flag duplicate implementations of the same docs-specific patterns. | ||
|
|
||
| 4. Docs typography and prose behavior | ||
| Review changes against the docs-specific typography stack and prose rules in `src/css/custom.css`, `tailwind.config.js`, and `src/theme/DocItem/index.js`. | ||
| Flag changes that break the docs body font override, code-font reset, heading anchor behavior, prose container sizing, or docs-specific gradient H1 treatment. | ||
|
|
||
| 5. Docs layout and responsiveness | ||
| Check whether changes preserve the established docs layout model: centered long-form content, Docusaurus doc shell, and homepage sections that fit within the existing `max-w-screen-lg` / `max-w-*` patterns. | ||
| Flag fixed-width layouts, desktop-only docs UI, or spacing that departs from the repo’s repeated docs patterns without reason. | ||
|
|
||
| 6. Docs accessibility and navigation behavior | ||
| Check icon links, toggle/filter controls, focus states, copy-button visibility, `aria-current` handling, and sidebar semantics. | ||
| Flag regressions in keyboard focus visibility, missing accessible names on icon-only actions, or changes that weaken Docusaurus navigation semantics. | ||
|
|
||
| 7. Docs-specific anti-patterns | ||
| Flag repo-local issues already present here and likely to spread: inline visual styles in React helpers, duplicated chip systems with local color maps, arbitrary-value classes for visual tokens, invalid JSX/SVG attributes, and mixing legacy `rounded-lg shadow-lg` cards into newer docs homepage sections without intent. | ||
| When a PR edits an older helper component, prefer small consistency fixes over opportunistic visual rewrites unless the PR is explicitly modernizing that surface. | ||
|
|
||
| How to deliver feedback | ||
| When reviewing a PR, structure feedback as: | ||
|
|
||
| Summary — one paragraph on docs-specific design health | ||
| Critical issues — docs-specific blockers that should be fixed before merge | ||
| Suggestions — non-blocking consistency improvements | ||
| Positive notes — at least one concrete thing done well | ||
|
|
||
| Reference actual file names and line numbers from the diff. Keep comments specific to this repo’s docs overrides rather than restating the shared commons. | ||
|
|
||
| Reference files | ||
| Read `references/docs-specific.md` for the repo-specific rules, tokens, components, and anti-patterns that apply only to `keploy/docs`. | ||
306 changes: 306 additions & 0 deletions
306
.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,306 @@ | ||
| # 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". | ||
|
|
||
| These examples are primarily copy-forward risks. Their presence in existing legacy files does not automatically mean every touched file should be rewritten unless the PR is already changing that surface in a meaningful way. | ||
|
|
||
| ## 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, prefer a class that references the theme variable: | ||
|
|
||
| ```jsx | ||
| <span className="text-[var(--ifm-color-primary)]" /> | ||
| ``` | ||
|
|
||
| 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` | ||
|
|
||
| Review note: | ||
|
|
||
| - Prefer not to spread this pattern into new work. | ||
| - If a PR only makes a small content edit inside one of these legacy sections, do not require a full card-system rewrite unless that mismatch is central to the review. | ||
|
|
||
| ## 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.