-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add Guidelines for components #70
Conversation
- This will be automatically handled by eslint and prettier now.
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.
Seems sensible enough, I guess :) Left a few small suggestions
guidelines/component-usage.md
Outdated
1. #### Naming Style **[⬆](#table-of-content)** | ||
|
||
* Always use the `.js` extension. | ||
* Use UpperCamelCase as a components name. |
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.
component's I guess? or "for component names", to match "for prop names" below.
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.
Right
guidelines/component-usage.md
Outdated
* Use UpperCamelCase as a components name. | ||
* Use lowerCamelCase for prop names. | ||
* Component name should be the same as the file name. This means that the use of `index` files is forbidden for components. It's easier to find components by filename rather than by folder name. | ||
* Component filenames should be in UpperCamelCase. It's allowed to use dots (`.`) to explicitly mention type of component: `Dashboard.component.js`. |
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.
What's the point of the dot notation? Also, you already say "Use UpperCamelCase as a components name" and "[The] Component name should be the same as the file name", so isn't this redundant?
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.
Makes sense. Removed
guidelines/component-usage.md
Outdated
* Always use the `.js` extension. | ||
* Use UpperCamelCase as a components name. | ||
* Use lowerCamelCase for prop names. | ||
* Component name should be the same as the file name. This means that the use of `index` files is forbidden for components. It's easier to find components by filename rather than by folder name. |
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.
Maybe "The component name"?
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.
Right
guidelines/component-usage.md
Outdated
* Component name should be the same as the file name. This means that the use of `index` files is forbidden for components. It's easier to find components by filename rather than by folder name. | ||
* Component filenames should be in UpperCamelCase. It's allowed to use dots (`.`) to explicitly mention type of component: `Dashboard.component.js`. | ||
* Props and state interface definitions shouldn't be prefixed with `I`. Use UpperCamelCase instead. | ||
* Exported props or state types should have a descriptive name and a `Props` or `State` suffix: `ProgressBarProps`, `EntityDialogState`. Non-exported types are recommended to be named `Props` and `State` respectively. |
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.
Maybe add "If more than one non-exported set of props is needed, give all but the main one a name matching their component"? At least in MB we have several cases where there's a main Props and then something like SmallComponentProps for a SmallComponent defined and used only on that same file.
A proper guideline for adding components has been added.