-
Notifications
You must be signed in to change notification settings - Fork 11
Add prompts for AI coding agents #2758
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
base: main
Are you sure you want to change the base?
Conversation
| ## Requirements vs Nimble | ||
|
|
||
| | | `nimble-components` | `spright-components` | | ||
| | ---------------------- | :-----------------: | :------------------: | | ||
| | Approved spec | 🟢 | 🟢 | | ||
| | Unit tests | 🟢 | 🟢 | | ||
| | Storybook visual tests | 🟢 | 🟢 | | ||
| | Storybook API docs | 🟢 | 🟢 | | ||
| | Storybook usage docs | 🟢 | 🟡 | | ||
| | Approved VxD\* | 🟢 | 🟡 | | ||
| | Approved IxD\* | 🟢 | 🟡 | | ||
| | Angular/Blazor support | 🟢 | 🟡 | | ||
| | Proper a11y | 🟢 | 🟡 | | ||
| | Minimal tech debt | 🟢 | 🟡 | | ||
| | Mobile support | 🟡 | 🟡 | | ||
|
|
||
| 🟢 = required, 🟡 = optional\*By an interaction and/or visual designer |
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.
outdated and not in this library, was moved to the root contributing. it should just not be duplicated
|
|
||
| The documentation and visual testing platform for Nimble. | ||
|
|
||
| - **Framework**: Storybook 7+ |
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.
We are on sb 10, should just say that..
| - Ensure custom elements are registered before use. | ||
| - Use `useLayoutEffect` to attach event listeners to custom events. | ||
| - Forward refs to the underlying DOM element. |
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.
first two are incorrect, those are not how wrappers are implemented. Should just point to like banner wrapper as an example
last one really only applies to the example app, and is just a statement of React's features. Don't see why we call out that specific react feature, seems unnecessary
|
|
||
| ## Common Pitfalls | ||
| - ❌ **`useEffect` for Events**: Use `useLayoutEffect` to avoid timing issues with custom elements. | ||
| - ❌ **Missing Ref Forwarding**: Wrappers must forward refs to the underlying web component. |
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.
I don't know what this means but no, our React wrappers don't forward ref explicitly. It's handled by our wrap helper.
| ## Common Pitfalls | ||
| - ❌ **`useEffect` for Events**: Use `useLayoutEffect` to avoid timing issues with custom elements. | ||
| - ❌ **Missing Ref Forwarding**: Wrappers must forward refs to the underlying web component. | ||
| - ❌ **ClassName Conflicts**: Ensure `className` prop is merged correctly. |
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.
This is just how React works. Don't see why we call out that specific react behavior, seems unnecessary
| | Area | Nimble | Ok | | ||
| | -------------------- | ------------------ | ------------------------------------------------- | | ||
| | Spec completeness | Required | Optional – document scope gaps in Storybook | | ||
| | Accessibility polish | Required | In-progress allowed, but note limitations | | ||
| | Framework wrappers | Required over time | Optional unless feature explicitly needs wrappers | | ||
| | Owner team | Nimble | Contributing feature team | |
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.
outdated, top-level contributing is up to date
|
|
||
| # Storybook + tests (run from repo root) | ||
| npm run storybook | ||
| npm run tdd -w @ni/nimble-components |
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.
this command was removed, npm run test-chrome -w @ni/nimble-components will do iterative component build and run tests
| 'agents.md', | ||
| 'docs/', | ||
| 'specs/', |
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.
add this to ignore these files in beachball change checks in packages
| - ❌ Styling component state via classes instead of attributes/behaviors. | ||
| - ❌ Hardcoding tag names inside templates instead of importing tag constants. | ||
| - ❌ Skipping Storybook docs/matrix updates when component APIs change. | ||
| - ❌ Not running formatter/tests before pushing (`npm run format`, `npm run tdd:watch`). |
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.
tdd:watch also removed, should probably just run the test command like above
|
|
||
| **Goal**: Synchronize the AI coding agent instructions with the latest project documentation, patterns, and VS Code best practices (using directory-scoped `agents.md` files). | ||
|
|
||
| **Role**: You are an expert maintainer of the Nimble Design System. Your job is to ensure that the instructions provided to AI agents are accurate, concise, up-to-date, and structured to create effective "Agents.md" instructions. |
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.
CONTRIBUTING should be updated with more detail on how humans are supposed to trigger this action.
Also. as commented in different files this is generating lots of incorrect content in agents.md and gets out of sync quickly. What is the workflow for updating those without having it conflict with this workflow running.
🤨 Rationale
Provides GitHub Copilot and other AI coding agents with essential context about the Nimble design system architecture, workflows, and conventions to enable more productive assistance.
👩💻 Implementation
Added covering:
🧪 Testing
N/A - Documentation only
✅ Checklist