diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa887929fb..fd94f18f85 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -189,89 +189,6 @@ give away to contributors - if you feel that Matrix-branded apparel is missing from your life, please mail us your shipping address to matrix at matrix.org and we'll try to fix it :) -## Sign off - -In order to have a concrete record that your contribution is intentional -and you agree to license it under the same terms as the project's license, we've -adopted the same lightweight approach that the Linux Kernel -(https://www.kernel.org/doc/html/latest/process/submitting-patches.html), Docker -(https://github.com/docker/docker/blob/master/CONTRIBUTING.md), and many other -projects use: the DCO (Developer Certificate of Origin: -http://developercertificate.org/). This is a simple declaration that you wrote -the contribution or otherwise have the right to contribute it to Matrix: - -``` -Developer Certificate of Origin -Version 1.1 - -Copyright (C) 2004, 2006 The Linux Foundation and its contributors. -660 York Street, Suite 102, -San Francisco, CA 94110 USA - -Everyone is permitted to copy and distribute verbatim copies of this -license document, but changing it is not allowed. - -Developer's Certificate of Origin 1.1 - -By making a contribution to this project, I certify that: - -(a) The contribution was created in whole or in part by me and I - have the right to submit it under the open source license - indicated in the file; or - -(b) The contribution is based upon previous work that, to the best - of my knowledge, is covered under an appropriate open source - license and I have the right under that license to submit that - work with modifications, whether created in whole or in part - by me, under the same open source license (unless I am - permitted to submit under a different license), as indicated - in the file; or - -(c) The contribution was provided directly to me by some other - person who certified (a), (b) or (c) and I have not modified - it. - -(d) I understand and agree that this project and the contribution - are public and that a record of the contribution (including all - personal information I submit with it, including my sign-off) is - maintained indefinitely and may be redistributed consistent with - this project or the open source license(s) involved. -``` - -If you agree to this for your contribution, then all that's needed is to -include the line in your commit or pull request comment: - -``` -Signed-off-by: Your Name -``` - -We accept contributions under a legally identifiable name, such as your name on -government documentation or common-law names (names claimed by legitimate usage -or repute). Unfortunately, we cannot accept anonymous contributions at this -time. - -Git allows you to add this signoff automatically when using the `-s` flag to -`git commit`, which uses the name and email set in your `user.name` and -`user.email` git configs. - -If you forgot to sign off your commits before making your pull request and are -on Git 2.17+ you can mass signoff using rebase: - -``` -git rebase --signoff origin/develop -``` - -## Private sign off - -If you would like to provide your legal name privately to the Matrix.org -Foundation (instead of in a public commit or comment), you can do so by emailing -your legal name and a link to the pull request to dco@matrix.org. It helps to -include "sign off" or similar in the subject line. You will then be instructed -further. - -Once private sign off is complete, doing so for future contributions will not -be required. - # Review expectations See https://github.com/element-hq/element-meta/wiki/Review-process diff --git a/README.md b/README.md index 5924f93498..00f8d6d89c 100644 --- a/README.md +++ b/README.md @@ -182,123 +182,11 @@ Dockerfile. # Development -Before attempting to develop on Element you **must** read the [developer guide -for `matrix-react-sdk`](https://github.com/matrix-org/matrix-react-sdk#developer-guide), which -also defines the design, architecture and style for Element too. +Please read through the following: -Read the [Choosing an issue](docs/choosing-an-issue.md) page for some guidance -about where to start. Before starting work on a feature, it's best to ensure -your plan aligns well with our vision for Element. Please chat with the team in -[#element-dev:matrix.org](https://matrix.to/#/#element-dev:matrix.org) before -you start so we can ensure it's something we'd be willing to merge. - -You should also familiarise yourself with the ["Here be Dragons" guide -](https://docs.google.com/document/d/12jYzvkidrp1h7liEuLIe6BMdU0NUjndUYI971O06ooM) -to the tame & not-so-tame dragons (gotchas) which exist in the codebase. - -The idea of Element is to be a relatively lightweight "skin" of customisations on -top of the underlying `matrix-react-sdk`. `matrix-react-sdk` provides both the -higher and lower level React components useful for building Matrix communication -apps using React. - -Please note that Element is intended to run correctly without access to the public -internet. So please don't depend on resources (JS libs, CSS, images, fonts) -hosted by external CDNs or servers but instead please package all dependencies -into Element itself. - -# Setting up a dev environment - -Much of the functionality in Element is actually in the `matrix-js-sdk` module. -It is possible to set these up in a way that makes it easy to track the `develop` branches -in git and to make local changes without having to manually rebuild each time. - -First clone and build `matrix-js-sdk`: - -```bash -git clone https://github.com/matrix-org/matrix-js-sdk.git -pushd matrix-js-sdk -yarn link -yarn install -popd -``` - -Clone the repo and switch to the `element-web` directory: - -```bash -git clone https://github.com/element-hq/element-web.git -cd element-web -``` - -Configure the app by copying `config.sample.json` to `config.json` and -modifying it. See the [configuration docs](docs/config.md) for details. - -Finally, build and start Element itself: - -```bash -yarn link matrix-js-sdk -yarn install -yarn start -``` - -Wait a few seconds for the initial build to finish; you should see something like: - -``` -[element-js] [webpack.Progress] 100% -[element-js] -[element-js] ℹ ｢wdm｣: 1840 modules -[element-js] ℹ ｢wdm｣: Compiled successfully. -``` - -Remember, the command will not terminate since it runs the web server -and rebuilds source files when they change. This development server also -disables caching, so do NOT use it in production. - -Open in your browser to see your newly built Element. - -**Note**: The build script uses inotify by default on Linux to monitor directories -for changes. If the inotify limits are too low your build will fail silently or with -`Error: EMFILE: too many open files`. To avoid these issues, we recommend a watch limit -of at least `128M` and instance limit around `512`. - -You may be interested in issues [#15750](https://github.com/element-hq/element-web/issues/15750) and -[#15774](https://github.com/element-hq/element-web/issues/15774) for further details. - -To set a new inotify watch and instance limit, execute: - -``` -sudo sysctl fs.inotify.max_user_watches=131072 -sudo sysctl fs.inotify.max_user_instances=512 -sudo sysctl -p -``` - -If you wish, you can make the new limits permanent, by executing: - -``` -echo fs.inotify.max_user_watches=131072 | sudo tee -a /etc/sysctl.conf -echo fs.inotify.max_user_instances=512 | sudo tee -a /etc/sysctl.conf -sudo sysctl -p -``` - ---- - -When you make changes to `matrix-js-sdk` they should be automatically picked up by webpack and built. - -If any of these steps error with, `file table overflow`, you are probably on a mac -which has a very low limit on max open files. Run `ulimit -Sn 1024` and try again. -You'll need to do this in each new terminal you open before building Element. - -## Running the tests - -There are a number of application-level tests in the `tests` directory; these -are designed to run with Jest and JSDOM. To run them - -``` -yarn test -``` - -### End-to-End tests - -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. +1. [Developer guide](./developer_guide.md) +2. [Code style](./code_style.md) +3. [Contribution guide](./CONTRIBUTING.md) # Translations diff --git a/code_style.md b/code_style.md index 9aa6836442..9f0501ccd8 100644 --- a/code_style.md +++ b/code_style.md @@ -5,15 +5,6 @@ adjacent to. As of writing, these are: - element-desktop - element-web -- matrix-js-sdk - -Other projects might extend this code style for increased strictness. For example, matrix-events-sdk -has stricter code organization to reduce the maintenance burden. These projects will declare their code -style within their own repos. - -Note that some requirements will be layer-specific. Where the requirements don't make sense for the -project, they are used to the best of their ability, used in spirit, or ignored if not applicable, -in that order. ## Guiding principles @@ -234,17 +225,19 @@ Unless otherwise specified, the following applies to all code: Inheriting all the rules of TypeScript, the following additionally apply: -1. Types for lifecycle functions are not required (render, componentDidMount, and so on). -2. Class components must always have a `Props` interface declared immediately above them. It can be +1. Component source files are named with upper camel case (e.g. views/rooms/EventTile.js) +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) +3. Types for lifecycle functions are not required (render, componentDidMount, and so on). +4. Class components must always have a `Props` interface declared immediately above them. It can be empty if the component accepts no props. -3. Class components should have an `State` interface declared immediately above them, but after `Props`. -4. Props and State should not be exported. Use `React.ComponentProps` +5. Class components should have an `State` interface declared immediately above them, but after `Props`. +6. Props and State should not be exported. Use `React.ComponentProps` instead. -5. One component per file, except when a component is a utility component specifically for the "primary" +7. One component per file, except when a component is a utility component specifically for the "primary" component. The utility component should not be exported. -6. Exported constants, enums, interfaces, functions, etc must be separate from files containing components +8. Exported constants, enums, interfaces, functions, etc must be separate from files containing components or stores. -7. Stores should use a singleton pattern with a static instance property: +9. Stores should use a singleton pattern with a static instance property: ```typescript class FooStore { @@ -261,44 +254,41 @@ Inheriting all the rules of TypeScript, the following additionally apply: } ``` -8. Stores must support using an alternative MatrixClient and dispatcher instance. -9. Utilities which require JSX must be split out from utilities which do not. This is to prevent import - cycles during runtime where components accidentally include more of the app than they intended. -10. Interdependence between stores should be kept to a minimum. Break functions and constants out to utilities +10. Stores must support using an alternative MatrixClient and dispatcher instance. +11. Utilities which require JSX must be split out from utilities which do not. This is to prevent import + cycles during runtime where components accidentally include more of the app than they intended. +12. Interdependence between stores should be kept to a minimum. Break functions and constants out to utilities if at all possible. -11. A component should only use CSS class names in line with the component name. +13. A component should only use CSS class names in line with the component name. 1. When knowingly using a class name from another component, document it with a [comment](#comments). -12. Curly braces within JSX should be padded with a space, however properties on those components should not. +14. Curly braces within JSX should be padded with a space, however properties on those components should not. See above code example. -13. Functions used as properties should either be defined on the class or stored in a variable. They should not +15. Functions used as properties should either be defined on the class or stored in a variable. They should not be inline unless mocking/short-circuiting the value. -14. Prefer hooks (functional components) over class components. Be consistent with the existing area if unsure +16. Prefer hooks (functional components) over class components. Be consistent with the existing area if unsure which should be used. 1. Unless the component is considered a "structure", in which case use classes. -15. Write more views than structures. Structures are chunks of functionality like MatrixChat while views are +17. Write more views than structures. Structures are chunks of functionality like MatrixChat while views are isolated components. -16. Components should serve a single, or near-single, purpose. -17. Prefer to derive information from component properties rather than establish state. -18. Do not use `React.Component::forceUpdate`. +18. Components should serve a single, or near-single, purpose. +19. Prefer to derive information from component properties rather than establish state. +20. Do not use `React.Component::forceUpdate`. ## Stylesheets (\*.pcss = PostCSS + Plugins) Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, but actually it is not. -1. Class names must be prefixed with "mx\_". -2. Class names must denote the component which defines them, followed by any context. - The context is not further specified here in terms of meaning or syntax. - Use whatever is appropriate for your implementation use case. - Some examples: - 1. `mx_MyFoo` - 2. `mx_MyFoo_avatar` - 3. `mx_MyFoo_avatarUser` - 4. `mx_MyFoo_avatar--user` -3. Use the `$font` variables instead of manual values. -4. Keep indentation/nesting to a minimum. Maximum suggested nesting is 5 layers. -5. Use the whole class name instead of shortcuts: +1. The view's CSS file MUST have the same name as the component (e.g. `view/rooms/_MessageTile.css` for `MessageTile.tsx` component). +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. +3. Class names must be prefixed with "mx\_". +4. Class names must strictly denote the component which defines them. + For example: `mx_MyFoo` for `MyFoo` component. +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. +6. Use the `$font` variables instead of manual values. +7. Keep indentation/nesting to a minimum. Maximum suggested nesting is 5 layers. +8. Use the whole class name instead of shortcuts: ```scss .mx_MyFoo { @@ -309,7 +299,7 @@ Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, b } ``` -6. Break multiple selectors over multiple lines this way: +9. Break multiple selectors over multiple lines this way: ```scss .mx_MyFoo, @@ -319,9 +309,9 @@ Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, b } ``` -7. Non-shared variables should use $lowerCamelCase. Shared variables use $dashed-naming. -8. Overrides to Z indexes, adjustments of dimensions/padding with pixels, and so on should all be - [documented](#comments) for what the values mean: +10. Non-shared variables should use $lowerCamelCase. Shared variables use $dashed-naming. +11. Overrides to Z indexes, adjustments of dimensions/padding with pixels, and so on should all be + [documented](#comments) for what the values mean: ```scss .mx_MyFoo { @@ -331,7 +321,9 @@ Note: We use PostCSS + some plugins to process our styles. It looks like SCSS, b } ``` -9. Avoid the use of `!important`. If `!important` is necessary, add a [comment](#comments) explaining why. +12. Avoid the use of `!important`. If `!important` is necessary, add a [comment](#comments) explaining why. +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. +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. ## Tests diff --git a/developer_guide.md b/developer_guide.md new file mode 100644 index 0000000000..fa4bb9a239 --- /dev/null +++ b/developer_guide.md @@ -0,0 +1,126 @@ +# Developer Guide + +## Development + +Read the [Choosing an issue](docs/choosing-an-issue.md) page for some guidance +about where to start. Before starting work on a feature, it's best to ensure +your plan aligns well with our vision for Element. Please chat with the team in +[#element-dev:matrix.org](https://matrix.to/#/#element-dev:matrix.org) before +you start so we can ensure it's something we'd be willing to merge. + +You should also familiarise yourself with the ["Here be Dragons" guide +](https://docs.google.com/document/d/12jYzvkidrp1h7liEuLIe6BMdU0NUjndUYI971O06ooM) +to the tame & not-so-tame dragons (gotchas) which exist in the codebase. + +Please note that Element is intended to run correctly without access to the public +internet. So please don't depend on resources (JS libs, CSS, images, fonts) +hosted by external CDNs or servers but instead please package all dependencies +into Element itself. + +## Setting up a dev environment + +Much of the functionality in Element is actually in the `matrix-js-sdk` module. +It is possible to set these up in a way that makes it easy to track the `develop` branches +in git and to make local changes without having to manually rebuild each time. + +First clone and build `matrix-js-sdk`: + +```bash +git clone https://github.com/matrix-org/matrix-js-sdk.git +pushd matrix-js-sdk +yarn link +yarn install +popd +``` + +Clone the repo and switch to the `element-web` directory: + +```bash +git clone https://github.com/element-hq/element-web.git +cd element-web +``` + +Configure the app by copying `config.sample.json` to `config.json` and +modifying it. See the [configuration docs](docs/config.md) for details. + +Finally, build and start Element itself: + +```bash +yarn link matrix-js-sdk +yarn install +yarn start +``` + +Wait a few seconds for the initial build to finish; you should see something like: + +``` +[element-js] [webpack.Progress] 100% +[element-js] +[element-js] ℹ ｢wdm｣: 1840 modules +[element-js] ℹ ｢wdm｣: Compiled successfully. +``` + +Remember, the command will not terminate since it runs the web server +and rebuilds source files when they change. This development server also +disables caching, so do NOT use it in production. + +Open in your browser to see your newly built Element. + +**Note**: The build script uses inotify by default on Linux to monitor directories +for changes. If the inotify limits are too low your build will fail silently or with +`Error: EMFILE: too many open files`. To avoid these issues, we recommend a watch limit +of at least `128M` and instance limit around `512`. + +You may be interested in issues [#15750](https://github.com/element-hq/element-web/issues/15750) and +[#15774](https://github.com/element-hq/element-web/issues/15774) for further details. + +To set a new inotify watch and instance limit, execute: + +``` +sudo sysctl fs.inotify.max_user_watches=131072 +sudo sysctl fs.inotify.max_user_instances=512 +sudo sysctl -p +``` + +If you wish, you can make the new limits permanent, by executing: + +``` +echo fs.inotify.max_user_watches=131072 | sudo tee -a /etc/sysctl.conf +echo fs.inotify.max_user_instances=512 | sudo tee -a /etc/sysctl.conf +sudo sysctl -p +``` + +--- + +When you make changes to `matrix-js-sdk` they should be automatically picked up by webpack and built. + +If any of these steps error with, `file table overflow`, you are probably on a mac +which has a very low limit on max open files. Run `ulimit -Sn 1024` and try again. +You'll need to do this in each new terminal you open before building Element. + +## Running the tests + +There are a number of application-level tests in the `tests` directory; these +are designed to run with Jest and JSDOM. To run them + +``` +yarn test +``` + +### End-to-End tests + +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. + +## General github guidelines + +1. **Pull requests must only be filed against the `develop` branch.** +2. Try to keep your pull requests concise. Split them up if necessary. +3. Ensure that you provide a description that explains the fix/feature and its intent. + +## Adding new code + +New code should be committed as follows: + +- All new components: https://github.com/element-hq/element-web/tree/develop/src/components +- CSS: https://github.com/element-hq/element-web/tree/develop/res/css +- Theme specific CSS & resources: https://github.com/element-hq/element-web/tree/develop/res/themes