diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 56bf691d3a..2f8575bc2f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,8 @@ Have questions about this document or anything not covered here? Feel free to re * [Build the user interface](#build-the-user-interface) * [Accessing the AWX web interface](#accessing-the-awx-web-interface) * [Working with React](#working-with-react) + * [App structure](#app-structure) + * [Naming files](#naming-files) * [Class constructors vs Class properties](#class-constructors-vs-class-properties) * [Binding](#binding) * [Typechecking with PropTypes](#typechecking-with-proptypes) @@ -56,6 +58,50 @@ Run the following to build the AWX UI: You can now log into the AWX web interface at [https://127.0.0.1:3001](https://127.0.0.1:3001). ## Working with React + +### App structure + +All source code lives in the `/src` directory and all tests live in the `/__tests__` directory (mimicing the internal structure of `/src`). + +Inside these folders, the internal structure is: +- **/components** - All generic components that are meant to be used in multiple contexts throughout awx. Things like buttons, tabs go here. +- **/contexts** - Components which utilize react's context api. +- **/pages** - Based on the various routes of awx. + - **/components** - Components that are meant to be used specifically by a particular route, but might be sharable across pages of that route. For example, a form component which is used on both add and edit screens. + - **/screens** - Individual pages of the route, such as add, edit, list, related lists, etc. +- **/util** - Stateless helper functions that aren't tied to react. + +#### Bootstrapping the application (root src/ files) + +In the root of `/src`, there are a few files which are used to initialize the react app. These are + +- **index.jsx** + - Connects react app to root dom node. + - Sets up root route structure, navigation grouping and login modal + - Calls base context providers + - Imports .scss styles. +- **app.jsx** + - Sets standard page layout, about modal, and root dialog modal. +- **RootProvider.jsx** + - Sets up all context providers. + - Initializes i18n. + +### Naming files + +Ideally, files should be named the same as the component they export, and tests with `.test` appended. In other words, `` would be defined in `FooBar.jsx`, and its tests would be defined in `FooBar.test.jsx`. + +#### Naming components that use the context api + +**File naming** - Since contexts export both consumer and provider (and potentially in withContext function form), the file can be simplified to be named after the consumer export. In other words, the file containing the `Network` context components would be named `Network.jsx`. + +**Component naming and conventions** - In order to provide a consistent interface with react-router and lingui, as well as make their usage easier and less verbose, context components follow these conventions: +- Providers are wrapped in a component in the `FooProvider` format. + - The value prop of the provider should be pulled from state. This is recommended by the react docs, [here](here). + - The provider should also be able to accept its value by prop for testing. + - Any sort of code related to grabbing data to put on the context should be done in this component. +- Consumers are wrapped in a component in the `Foo` format. +- If it makes sense, consumers can be exported as a function in the `withFoo()` format. If a component is wrapped in this function, its context values are available on the component as props. + ### Class constructors vs Class properties It is good practice to use constructor-bound instance methods rather than methods as class properties. Methods as arrow functions provide lexical scope and are bound to the Component class instance instead of the class itself. This makes it so we cannot easily test a Component's methods without invoking an instance of the Component and calling the method directly within our tests. diff --git a/__tests__/qs.test.js b/__tests__/util/qs.test.js similarity index 93% rename from __tests__/qs.test.js rename to __tests__/util/qs.test.js index 2016f49b45..01178e4b17 100644 --- a/__tests__/qs.test.js +++ b/__tests__/util/qs.test.js @@ -1,4 +1,4 @@ -import { encodeQueryString, parseQueryString } from '../src/qs'; +import { encodeQueryString, parseQueryString } from '../../src/util/qs'; describe('qs (qs.js)', () => { test('encodeQueryString returns the expected queryString', () => { diff --git a/src/components/NotificationsList/Notifications.list.jsx b/src/components/NotificationsList/Notifications.list.jsx index ac20a8363f..cd3ea675b7 100644 --- a/src/components/NotificationsList/Notifications.list.jsx +++ b/src/components/NotificationsList/Notifications.list.jsx @@ -14,7 +14,7 @@ import DataListToolbar from '../DataListToolbar'; import NotificationListItem from './NotificationListItem'; import Pagination from '../Pagination'; -import { parseQueryString } from '../../qs'; +import { parseQueryString } from '../../util/qs'; class Notifications extends Component { columns = [ diff --git a/src/qs.js b/src/util/qs.js similarity index 100% rename from src/qs.js rename to src/util/qs.js