Skip to content

Commit 58b0731

Browse files
committed
Add additional best practices to guide
1 parent ba7ffc6 commit 58b0731

File tree

2 files changed

+92
-6
lines changed

2 files changed

+92
-6
lines changed

.eslintrc.js

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// https://eslint.org/
12
module.exports = {
23
parser: "babel-eslint",
34
env: {
@@ -11,19 +12,26 @@ module.exports = {
1112
NAF: true
1213
},
1314
plugins: ["prettier", "react", "react-hooks", "@calm/react-intl"],
15+
16+
// https://eslint.org/docs/rules/
1417
rules: {
18+
// https://github.com/prettier/eslint-plugin-prettier
1519
"prettier/prettier": "error",
20+
1621
"prefer-const": "error",
1722
"no-use-before-define": "error",
1823
"no-var": "error",
1924
"no-throw-literal": "error",
2025
// Light console usage is useful but remove debug logs before merging to master.
2126
"no-console": "off",
27+
28+
// https://www.npmjs.com/package/eslint-plugin-react-hooks
2229
"react-hooks/rules-of-hooks": "error",
2330
"react-hooks/exhaustive-deps": "warn",
24-
// TODO: Move to throwing lint errors for react-intl once migration is complete
31+
32+
// https://github.com/calm/eslint-plugin-react-intl
2533
"@calm/react-intl/missing-formatted-message": [
26-
"warn",
34+
"error",
2735
{
2836
noTrailingWhitespace: true,
2937
ignoreLinks: true,
@@ -33,7 +41,7 @@ module.exports = {
3341
}
3442
],
3543
"@calm/react-intl/missing-attribute": [
36-
"warn",
44+
"error",
3745
{
3846
noTrailingWhitespace: true,
3947
noSpreadOperator: true,
@@ -43,7 +51,12 @@ module.exports = {
4351
requireDefaultMessage: true
4452
}
4553
],
46-
"@calm/react-intl/missing-values": "warn"
54+
"@calm/react-intl/missing-values": "error"
4755
},
48-
extends: ["prettier", "plugin:react/recommended", "eslint:recommended"]
56+
extends: [
57+
// https://github.com/prettier/eslint-config-prettier
58+
"prettier",
59+
"plugin:react/recommended",
60+
"eslint:recommended"
61+
]
4962
};

doc/best-practices.md

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,29 @@
11
# Hubs Frontend Development Best Practices
22

3+
The Hubs UI is built with React, CSS Modules, and a number of other libraries. This guide is intended to get you up to speed with how we on the Hubs team write frontend code.
4+
5+
## Javascript Style
6+
7+
We use [Prettier](https://prettier.io/) for code formatting and a series of [ESLint](https://eslint.org/) rules for linting.
8+
9+
Currently the only modification we have made to the default prettier config is a 120 character line width. We as a team decided that 80 characters was not enough despite the Prettier team's [strong suggestion](https://prettier.io/docs/en/options.html#print-width).
10+
11+
You can look at our [.eslintrc.js](../.eslintrc.js) file for our linting rules. Documentation for the various rules are linked in that file.
12+
313
## React
414

15+
Hubs underwent a major redesign in 2020 and was transitioned from React class based components to React hooks in the process. There may still be some legacy code that uses class based components, but we intend to use hooks for all evergreen code.
16+
17+
### React Resources
18+
- [Official React Docs](https://reactjs.org/docs/getting-started.html)
19+
20+
### Hooks
21+
22+
If you are new to React hooks we recommend the following guides to get started:
23+
- [Official React Docs on Hooks](https://reactjs.org/docs/hooks-intro.html)
24+
- [Thinking in React Hooks](https://wattenberger.com/blog/react-hooks)
25+
26+
527
### File / Folder Structure
628

729
- Keep files relatively small
@@ -33,4 +55,55 @@
3355
- Wiring business logic to presentational components should be done in container components
3456
- Container components can depend on presentational components, hooks with business logic, and other container components.
3557
- Container components shouldn't contain html elements or their own stylesheets
36-
-
58+
59+
## CSS / CSS Modules
60+
61+
Hubs uses [CSS Modules](https://github.com/css-modules/css-modules) and [SASS](https://sass-lang.com/) for styles.
62+
63+
You should avoid global class names whenever possible and instead rely on imported classes from css modules in your code.
64+
65+
SASS mixins, functions, etc. should generally be avoided whenever possible. They add complexity to stylesheets that makes them harder to read and understand.
66+
67+
You should avoid nested SASS rules when possible. One major reason for using them is for [selector specificity](https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity). Another would be for programatically changing a single parent class that affects multiple child classes. Overuse of nested selectors bloats our stylesheets and should be avoided.
68+
69+
We use "t-shirt size" class suffixes for utility classes. Follow this pattern whenever you need to specify programatic sizes.
70+
71+
Ex.
72+
```css
73+
.button-2xs {}
74+
.button-xs {}
75+
.button-sm {}
76+
.button-md {}
77+
.button-lg {}
78+
.button-xl {}
79+
.button-2xl {}
80+
```
81+
82+
All CSS colors and fonts should be defined in [theme.scss](../src/react-components/styles/theme.scss). These variables are configurable in Hubs Cloud instances. In some rare cases, you may want to explicitly enforce a color that cannot be configured (Ex. black/white text for an overlay).
83+
84+
Theme variables are imported using the `@use` keyword. This must be the first line in your `.scss` file. All variables will be namespaced with the filename. For `theme.scss` this would be `theme.$my-variable`.
85+
86+
Ex.
87+
88+
```scss
89+
@use '../styles/theme';
90+
/* rest of the styles */
91+
```
92+
93+
CSS styles should be written with mobile-first media queries. This means the styles in the bare class should represent those on the smallest screen and media queries should be used for styles that are different on larger screens.
94+
95+
Ex.
96+
```scss
97+
:local(.sidebar) {
98+
position: relative;
99+
display: flex;
100+
flex-direction: column;
101+
height: 100%;
102+
background-color: theme.$white;
103+
pointer-events: auto;
104+
105+
@media(min-width: theme.$breakpoint-lg) and (min-height: theme.$breakpoint-vr) {
106+
border-left: 1px solid theme.$lightgrey;
107+
}
108+
}
109+
```

0 commit comments

Comments
 (0)