Conversation
akademy
left a comment
akademy
left a comment
There was a problem hiding this comment.
Sorry, did a re-review on the contributing.md file.
Mostly, everything is fine.....
| labels: ["type:rfc", "status:needs-triage"] | ||
| --- | ||
|
|
||
| # RFC: [Title] |
There was a problem hiding this comment.
I don't know what rfc means... maybe we could just put the full name here?
| @@ -1,131 +1,134 @@ | |||
| SciReactUI Changelog | |||
| ==================== | |||
There was a problem hiding this comment.
Do we have to lose this formatting? It is far clearer this way.
| - An *auth* parameter was added to *User* to simplify when *AuthProvider* is used. | ||
| - *ScrollableImages* can now display in a wide view, with multiple images. | ||
|
|
||
| - New _Progress_ component based on Diamond Light added. |
There was a problem hiding this comment.
Why Progress and not Progress?!
|
|
||
| ## Project Principles | ||
|
|
||
| - **Quality over quantity**: stable, well‑tested, well‑documented components. |
There was a problem hiding this comment.
I don't think we have well-documented components in most cases.
| Not in scope: | ||
|
|
||
| - Application‑specific business logic. | ||
| - Utilities unrelated to UI components. |
There was a problem hiding this comment.
Actually, I already stuck in a auth component... which while it does interact with UI bits, it doesn't itself provide ui... and it is technically business logic, even if it is generic.
| - Understand every line submitted. | ||
| - No unverifiable AI‑generated content. | ||
| - Validate types, accessibility, and performance. | ||
| - Never include secrets or internal prompts. | ||
| - No emojis. |
There was a problem hiding this comment.
This section is a weird collection of things. Most already covered. And what is with emojis?!
| - Understand every line submitted. | |
| - No unverifiable AI‑generated content. | |
| - Validate types, accessibility, and performance. | |
| - Never include secrets or internal prompts. | |
| - No emojis. | |
| - Understand every line submitted. | |
| - No unverifiable AI‑generated content. | |
| - Never include prompts. |
| ### Visual Regression Testing | ||
|
|
||
| Automated comparison detecting unintended visual changes between versions. |
There was a problem hiding this comment.
This is never used in the document... I bet half of the rest of this section isn't either....
| Examples: | ||
|
|
||
| - New interactive components (Select, Tabs, Dialog, DatePicker…) | ||
| - New primitives (tokens, foundational elements) | ||
| - Multi‑part components with complex behaviour |
There was a problem hiding this comment.
Is it really necessary to explain what a component is!
| 1. **Open an _RFC_ issue.** | ||
| Must include: problem, use‑cases, [API design](#api-design), [accessibility model](#accessibility-model-a11y-model), states, variants, edge cases, [interaction model](#interaction-model), alternatives considered. | ||
|
|
||
| 2. **Achieve functional consensus.** |
|
|
||
| ## Pull Requests | ||
|
|
||
| Before opening a PR: |
There was a problem hiding this comment.
| Before opening a PR: | |
| Before writing any code: |
| - Use [design tokens](#design-tokens) for spacing, colour, typography, motion. | ||
| - Prefer semantic HTML over ARIA where possible. | ||
| - Support `className` and `style` passthrough. | ||
| - Use `forwardRef`. |
There was a problem hiding this comment.
forwardRef has been deprecated https://react.dev/reference/react/forwardRef
|
|
||
| ## Testing | ||
|
|
||
| - **Unit tests**: React Testing Library + Jest/Vitest. |
There was a problem hiding this comment.
Maybe we should specify it should be Vitest, and not Jest
| - Enhancements, new [variants](#variants) → _Proposal_ | ||
| - New components or major changes → _RFC_ | ||
|
|
||
| Discussion must reach **[functional consensus](#functional-consensus)** before design or code begins. |
There was a problem hiding this comment.
I think this is going to be a massive hurdle for anyone who has already developed a component for use in their own apps, and just wants to share something that is already tested, in use, and designed. Especially because code has already been started (and completed, as a matter of fact)
Instead, if something already exists and is production ready, would it be best to create the PR and allow for discussion (design, functional, etc.) inside the PR?
|
|
||
| Before opening a PR: | ||
|
|
||
| - Ensure an issue exists and consensus is reached. |
There was a problem hiding this comment.
Should this only be the case for components that don't yet exist? Because if we have a component that already exists in a separate app, and a branch is created, that allows maintainers to verify changes more easily, as opposed to screenshots/verbal descriptions of how the component works
3 participants
Adds a contributing guide and proposed templates for bug report, proposal and rfc.