Skip to content

Commit 3c690e6

Browse files
Add code_style, developer_guide and fix CONTRIBUTING.md (#29210)
* Remove reference to matrix-js-sdk in code_style.md * Absorb exisiting documentation from matrix-react-sdk * Crete a separate developer guide * Remove sign-off from CONTRIBUTING.md Since sign-off is irrelevant to element-web repo with the introduction of CLA. * Link to the new docs in README * Elaborate on the rule * Fix lint
1 parent 53f8312 commit 3c690e6

File tree

4 files changed

+167
-244
lines changed

4 files changed

+167
-244
lines changed

CONTRIBUTING.md

Lines changed: 0 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -189,89 +189,6 @@ give away to contributors - if you feel that Matrix-branded apparel is missing
189189
from your life, please mail us your shipping address to matrix at matrix.org
190190
and we'll try to fix it :)
191191

192-
## Sign off
193-
194-
In order to have a concrete record that your contribution is intentional
195-
and you agree to license it under the same terms as the project's license, we've
196-
adopted the same lightweight approach that the Linux Kernel
197-
(https://www.kernel.org/doc/html/latest/process/submitting-patches.html), Docker
198-
(https://github.com/docker/docker/blob/master/CONTRIBUTING.md), and many other
199-
projects use: the DCO (Developer Certificate of Origin:
200-
http://developercertificate.org/). This is a simple declaration that you wrote
201-
the contribution or otherwise have the right to contribute it to Matrix:
202-
203-
```
204-
Developer Certificate of Origin
205-
Version 1.1
206-
207-
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
208-
660 York Street, Suite 102,
209-
San Francisco, CA 94110 USA
210-
211-
Everyone is permitted to copy and distribute verbatim copies of this
212-
license document, but changing it is not allowed.
213-
214-
Developer's Certificate of Origin 1.1
215-
216-
By making a contribution to this project, I certify that:
217-
218-
(a) The contribution was created in whole or in part by me and I
219-
have the right to submit it under the open source license
220-
indicated in the file; or
221-
222-
(b) The contribution is based upon previous work that, to the best
223-
of my knowledge, is covered under an appropriate open source
224-
license and I have the right under that license to submit that
225-
work with modifications, whether created in whole or in part
226-
by me, under the same open source license (unless I am
227-
permitted to submit under a different license), as indicated
228-
in the file; or
229-
230-
(c) The contribution was provided directly to me by some other
231-
person who certified (a), (b) or (c) and I have not modified
232-
it.
233-
234-
(d) I understand and agree that this project and the contribution
235-
are public and that a record of the contribution (including all
236-
personal information I submit with it, including my sign-off) is
237-
maintained indefinitely and may be redistributed consistent with
238-
this project or the open source license(s) involved.
239-
```
240-
241-
If you agree to this for your contribution, then all that's needed is to
242-
include the line in your commit or pull request comment:
243-
244-
```
245-
Signed-off-by: Your Name <[email protected]>
246-
```
247-
248-
We accept contributions under a legally identifiable name, such as your name on
249-
government documentation or common-law names (names claimed by legitimate usage
250-
or repute). Unfortunately, we cannot accept anonymous contributions at this
251-
time.
252-
253-
Git allows you to add this signoff automatically when using the `-s` flag to
254-
`git commit`, which uses the name and email set in your `user.name` and
255-
`user.email` git configs.
256-
257-
If you forgot to sign off your commits before making your pull request and are
258-
on Git 2.17+ you can mass signoff using rebase:
259-
260-
```
261-
git rebase --signoff origin/develop
262-
```
263-
264-
## Private sign off
265-
266-
If you would like to provide your legal name privately to the Matrix.org
267-
Foundation (instead of in a public commit or comment), you can do so by emailing
268-
your legal name and a link to the pull request to [email protected]. It helps to
269-
include "sign off" or similar in the subject line. You will then be instructed
270-
further.
271-
272-
Once private sign off is complete, doing so for future contributions will not
273-
be required.
274-
275192
# Review expectations
276193

277194
See https://github.com/element-hq/element-meta/wiki/Review-process

README.md

Lines changed: 4 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -182,123 +182,11 @@ Dockerfile.
182182

183183
# Development
184184

185-
Before attempting to develop on Element you **must** read the [developer guide
186-
for `matrix-react-sdk`](https://github.com/matrix-org/matrix-react-sdk#developer-guide), which
187-
also defines the design, architecture and style for Element too.
188-
189-
Read the [Choosing an issue](docs/choosing-an-issue.md) page for some guidance
190-
about where to start. Before starting work on a feature, it's best to ensure
191-
your plan aligns well with our vision for Element. Please chat with the team in
192-
[#element-dev:matrix.org](https://matrix.to/#/#element-dev:matrix.org) before
193-
you start so we can ensure it's something we'd be willing to merge.
194-
195-
You should also familiarise yourself with the ["Here be Dragons" guide
196-
](https://docs.google.com/document/d/12jYzvkidrp1h7liEuLIe6BMdU0NUjndUYI971O06ooM)
197-
to the tame & not-so-tame dragons (gotchas) which exist in the codebase.
198-
199-
The idea of Element is to be a relatively lightweight "skin" of customisations on
200-
top of the underlying `matrix-react-sdk`. `matrix-react-sdk` provides both the
201-
higher and lower level React components useful for building Matrix communication
202-
apps using React.
203-
204-
Please note that Element is intended to run correctly without access to the public
205-
internet. So please don't depend on resources (JS libs, CSS, images, fonts)
206-
hosted by external CDNs or servers but instead please package all dependencies
207-
into Element itself.
208-
209-
# Setting up a dev environment
210-
211-
Much of the functionality in Element is actually in the `matrix-js-sdk` module.
212-
It is possible to set these up in a way that makes it easy to track the `develop` branches
213-
in git and to make local changes without having to manually rebuild each time.
214-
215-
First clone and build `matrix-js-sdk`:
216-
217-
```bash
218-
git clone https://github.com/matrix-org/matrix-js-sdk.git
219-
pushd matrix-js-sdk
220-
yarn link
221-
yarn install
222-
popd
223-
```
224-
225-
Clone the repo and switch to the `element-web` directory:
226-
227-
```bash
228-
git clone https://github.com/element-hq/element-web.git
229-
cd element-web
230-
```
231-
232-
Configure the app by copying `config.sample.json` to `config.json` and
233-
modifying it. See the [configuration docs](docs/config.md) for details.
234-
235-
Finally, build and start Element itself:
236-
237-
```bash
238-
yarn link matrix-js-sdk
239-
yarn install
240-
yarn start
241-
```
242-
243-
Wait a few seconds for the initial build to finish; you should see something like:
244-
245-
```
246-
[element-js] <s> [webpack.Progress] 100%
247-
[element-js]
248-
[element-js] ℹ 「wdm」: 1840 modules
249-
[element-js] ℹ 「wdm」: Compiled successfully.
250-
```
251-
252-
Remember, the command will not terminate since it runs the web server
253-
and rebuilds source files when they change. This development server also
254-
disables caching, so do NOT use it in production.
255-
256-
Open <http://127.0.0.1:8080/> in your browser to see your newly built Element.
257-
258-
**Note**: The build script uses inotify by default on Linux to monitor directories
259-
for changes. If the inotify limits are too low your build will fail silently or with
260-
`Error: EMFILE: too many open files`. To avoid these issues, we recommend a watch limit
261-
of at least `128M` and instance limit around `512`.
262-
263-
You may be interested in issues [#15750](https://github.com/element-hq/element-web/issues/15750) and
264-
[#15774](https://github.com/element-hq/element-web/issues/15774) for further details.
265-
266-
To set a new inotify watch and instance limit, execute:
267-
268-
```
269-
sudo sysctl fs.inotify.max_user_watches=131072
270-
sudo sysctl fs.inotify.max_user_instances=512
271-
sudo sysctl -p
272-
```
273-
274-
If you wish, you can make the new limits permanent, by executing:
275-
276-
```
277-
echo fs.inotify.max_user_watches=131072 | sudo tee -a /etc/sysctl.conf
278-
echo fs.inotify.max_user_instances=512 | sudo tee -a /etc/sysctl.conf
279-
sudo sysctl -p
280-
```
281-
282-
---
283-
284-
When you make changes to `matrix-js-sdk` they should be automatically picked up by webpack and built.
285-
286-
If any of these steps error with, `file table overflow`, you are probably on a mac
287-
which has a very low limit on max open files. Run `ulimit -Sn 1024` and try again.
288-
You'll need to do this in each new terminal you open before building Element.
289-
290-
## Running the tests
291-
292-
There are a number of application-level tests in the `tests` directory; these
293-
are designed to run with Jest and JSDOM. To run them
294-
295-
```
296-
yarn test
297-
```
298-
299-
### End-to-End tests
185+
Please read through the following:
300186

301-
See [matrix-react-sdk](https://github.com/matrix-org/matrix-react-sdk/#end-to-end-tests) for how to run the end-to-end tests.
187+
1. [Developer guide](./developer_guide.md)
188+
2. [Code style](./code_style.md)
189+
3. [Contribution guide](./CONTRIBUTING.md)
302190

303191
# Translations
304192

code_style.md

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,6 @@ adjacent to. As of writing, these are:
55

66
- element-desktop
77
- element-web
8-
- matrix-js-sdk
9-
10-
Other projects might extend this code style for increased strictness. For example, matrix-events-sdk
11-
has stricter code organization to reduce the maintenance burden. These projects will declare their code
12-
style within their own repos.
13-
14-
Note that some requirements will be layer-specific. Where the requirements don't make sense for the
15-
project, they are used to the best of their ability, used in spirit, or ignored if not applicable,
16-
in that order.
178

189
## Guiding principles
1910

@@ -234,17 +225,19 @@ Unless otherwise specified, the following applies to all code:
234225

235226
Inheriting all the rules of TypeScript, the following additionally apply:
236227

237-
1. Types for lifecycle functions are not required (render, componentDidMount, and so on).
238-
2. Class components must always have a `Props` interface declared immediately above them. It can be
228+
1. Component source files are named with upper camel case (e.g. views/rooms/EventTile.js)
229+
2. They are organised in a typically two-level hierarchy - first whether the component is a view or a structure, and then a broad functional grouping (e.g. 'rooms' here)
230+
3. Types for lifecycle functions are not required (render, componentDidMount, and so on).
231+
4. Class components must always have a `Props` interface declared immediately above them. It can be
239232
empty if the component accepts no props.
240-
3. Class components should have an `State` interface declared immediately above them, but after `Props`.
241-
4. Props and State should not be exported. Use `React.ComponentProps<typeof ComponentNameHere>`
233+
5. Class components should have an `State` interface declared immediately above them, but after `Props`.
234+
6. Props and State should not be exported. Use `React.ComponentProps<typeof ComponentNameHere>`
242235
instead.
243-
5. One component per file, except when a component is a utility component specifically for the "primary"
236+
7. One component per file, except when a component is a utility component specifically for the "primary"
244237
component. The utility component should not be exported.
245-
6. Exported constants, enums, interfaces, functions, etc must be separate from files containing components
238+
8. Exported constants, enums, interfaces, functions, etc must be separate from files containing components
246239
or stores.
247-
7. Stores should use a singleton pattern with a static instance property:
240+
9. Stores should use a singleton pattern with a static instance property:
248241

249242
```typescript
250243
class FooStore {
@@ -261,44 +254,41 @@ Inheriting all the rules of TypeScript, the following additionally apply:
261254
}
262255
```
263256

264-
8. Stores must support using an alternative MatrixClient and dispatcher instance.
265-
9. Utilities which require JSX must be split out from utilities which do not. This is to prevent import
266-
cycles during runtime where components accidentally include more of the app than they intended.
267-
10. Interdependence between stores should be kept to a minimum. Break functions and constants out to utilities
257+
10. Stores must support using an alternative MatrixClient and dispatcher instance.
258+
11. Utilities which require JSX must be split out from utilities which do not. This is to prevent import
259+
cycles during runtime where components accidentally include more of the app than they intended.
260+
12. Interdependence between stores should be kept to a minimum. Break functions and constants out to utilities
268261
if at all possible.
269-
11. A component should only use CSS class names in line with the component name.
262+
13. A component should only use CSS class names in line with the component name.
270263

271264
1. When knowingly using a class name from another component, document it with a [comment](#comments).
272265

273-
12. Curly braces within JSX should be padded with a space, however properties on those components should not.
266+
14. Curly braces within JSX should be padded with a space, however properties on those components should not.
274267
See above code example.
275-
13. Functions used as properties should either be defined on the class or stored in a variable. They should not
268+
15. Functions used as properties should either be defined on the class or stored in a variable. They should not
276269
be inline unless mocking/short-circuiting the value.
277-
14. Prefer hooks (functional components) over class components. Be consistent with the existing area if unsure
270+
16. Prefer hooks (functional components) over class components. Be consistent with the existing area if unsure
278271
which should be used.
279272
1. Unless the component is considered a "structure", in which case use classes.
280-
15. Write more views than structures. Structures are chunks of functionality like MatrixChat while views are
273+
17. Write more views than structures. Structures are chunks of functionality like MatrixChat while views are
281274
isolated components.
282-
16. Components should serve a single, or near-single, purpose.
283-
17. Prefer to derive information from component properties rather than establish state.
284-
18. Do not use `React.Component::forceUpdate`.
275+
18. Components should serve a single, or near-single, purpose.
276+
19. Prefer to derive information from component properties rather than establish state.
277+
20. Do not use `React.Component::forceUpdate`.
285278

286279
## Stylesheets (\*.pcss = PostCSS + Plugins)
287280

288281
Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, but actually it is not.
289282

290-
1. Class names must be prefixed with "mx\_".
291-
2. Class names must denote the component which defines them, followed by any context.
292-
The context is not further specified here in terms of meaning or syntax.
293-
Use whatever is appropriate for your implementation use case.
294-
Some examples:
295-
1. `mx_MyFoo`
296-
2. `mx_MyFoo_avatar`
297-
3. `mx_MyFoo_avatarUser`
298-
4. `mx_MyFoo_avatar--user`
299-
3. Use the `$font` variables instead of manual values.
300-
4. Keep indentation/nesting to a minimum. Maximum suggested nesting is 5 layers.
301-
5. Use the whole class name instead of shortcuts:
283+
1. The view's CSS file MUST have the same name as the component (e.g. `view/rooms/_MessageTile.css` for `MessageTile.tsx` component).
284+
2. Per-view CSS is optional - it could choose to inherit all its styling from the context of the rest of the app, although this is unusual.
285+
3. Class names must be prefixed with "mx\_".
286+
4. Class names must strictly denote the component which defines them.
287+
For example: `mx_MyFoo` for `MyFoo` component.
288+
5. Class names for DOM elements within a view which aren't components are named by appending a lower camel case identifier to the view's class name - e.g. .mx_MyFoo_randomDiv is how you'd name the class of an arbitrary div within the MyFoo view.
289+
6. Use the `$font` variables instead of manual values.
290+
7. Keep indentation/nesting to a minimum. Maximum suggested nesting is 5 layers.
291+
8. Use the whole class name instead of shortcuts:
302292

303293
```scss
304294
.mx_MyFoo {
@@ -309,7 +299,7 @@ Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, b
309299
}
310300
```
311301

312-
6. Break multiple selectors over multiple lines this way:
302+
9. Break multiple selectors over multiple lines this way:
313303

314304
```scss
315305
.mx_MyFoo,
@@ -319,9 +309,9 @@ Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, b
319309
}
320310
```
321311

322-
7. Non-shared variables should use $lowerCamelCase. Shared variables use $dashed-naming.
323-
8. Overrides to Z indexes, adjustments of dimensions/padding with pixels, and so on should all be
324-
[documented](#comments) for what the values mean:
312+
10. Non-shared variables should use $lowerCamelCase. Shared variables use $dashed-naming.
313+
11. Overrides to Z indexes, adjustments of dimensions/padding with pixels, and so on should all be
314+
[documented](#comments) for what the values mean:
325315

326316
```scss
327317
.mx_MyFoo {
@@ -331,7 +321,9 @@ Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, b
331321
}
332322
```
333323

334-
9. Avoid the use of `!important`. If `!important` is necessary, add a [comment](#comments) explaining why.
324+
12. Avoid the use of `!important`. If `!important` is necessary, add a [comment](#comments) explaining why.
325+
13. The CSS for a component can override the rules for child components. For instance, .mxRoomList .mx_RoomTile {} would be the selector to override styles of RoomTiles when viewed in the context of a RoomList view. Overrides must be scoped to the View's CSS class - i.e. don't just define .mx_RoomTile {} in RoomList.css - only RoomTile.css is allowed to define its own CSS. Instead, say .mx_RoomList .mx_RoomTile {} to scope the override only to the context of RoomList views. N.B. overrides should be relatively rare as in general CSS inheritance should be enough.
326+
14. Components should render only within the bounding box of their outermost DOM element. Page-absolute positioning and negative CSS margins and similar are generally not cool and stop the component from being reused easily in different places.
335327

336328
## Tests
337329

0 commit comments

Comments
 (0)