Skip to content

Commit

Permalink
Merge pull request #33 from centerofci/fe-standards
Browse files Browse the repository at this point in the history
Add three new front-end coding standards
  • Loading branch information
pavish committed Feb 4, 2022
2 parents b96ae98 + ba4c1a1 commit a6a0b92
Showing 1 changed file with 99 additions and 64 deletions.
163 changes: 99 additions & 64 deletions engineering/architecture/front-end-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ dateCreated: 2022-01-14T19:02:19.924Z

# Front end code standards

## Naming conventions
## General

### Naming conventions

* File names for Components, Classes and Stylesheets should be in PascalCase. Examples:

Expand Down Expand Up @@ -79,7 +81,92 @@ dateCreated: 2022-01-14T19:02:19.924Z

- [discussion](https://github.com/centerofci/mathesar/discussions/872)

## `null` vs `undefined`
## HTML

### Build components for valid DOM nesting

When working inside a component that _might_ be placed where [phrasing content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#phrasing_content) is required, be sure to only use phrasing content elements (like `span`) instead of _not_-phrasing content elements (like `div`).

- ✅ Good

`FancyCheckbox.svelte`

```ts
<span class="make-it-fancy">
<input type="checkbox" />
</span>
```

- ❌ Bad because it uses `div` instead of `span`

`FancyCheckbox.svelte`

```ts
<div class="make-it-fancy">
<input type="checkbox" />
</div>
```

Rationale:

- For example, let's build on the "Bad" example above and write the following Svelte code:

```svelte
<label>
<FancyCheckbox />
Foo
</label>
```

That Svelte code will render:

```html
<label>
<div class="make-it-fancy">
<input type="checkbox" />
</div>
Foo
</label>
```

That markup has invalid DOM nesting because a `label` element can only contain phrasing content -- but a `div` is _not_ phrasing content.

Tips:

- You can make a `span` _act_ like a `div` by setting `display: block` if needed.

Notes:

- Not all of our components adhere to this guideline yet.

## CSS

### CSS units

- Don't use `px` — use `rem` or `em` instead.

Exceptional cases where `px` is okay:

- when setting the root `font-size`

Note: some of our older code still does not conform to this standard.

### Component spacing and layout

Components should not set any space around their outer-most visual edges — instead the consuming component should be responsible for layout and spacing.

- **margin**: The component's root element should not set any margin.

- **padding**: If the component's root element has border, it's fine to set padding because the border will serve as the outer-most visual edge. But if there's no border, then there should be no padding.

## TypeScript

### `type` vs `interface`

- Prefer `interface` when possible.
- Use `type` when necessary.

### `null` vs `undefined`

Prefer using `undefined` over `null` where possible.

Expand Down Expand Up @@ -133,7 +220,15 @@ Additional context:

- [discussion](https://github.com/centerofci/mathesar/discussions/825)

## Minimize Svelte store instances
### API type definitions

TypeScript types (including interfaces) which describe API requests and responses (and properties therein) should be separated from other front end code and placed in the `mathesar_ui/src/api` directory. This structure serves to communicate that, unlike other TypeScript types, the front end does not have control over the API types.

[Discussion](https://github.com/centerofci/mathesar/discussions/875)

## Svelte

### Minimize Svelte store instances

- ✅ Good because only one `cost` store is created.

Expand Down Expand Up @@ -173,69 +268,9 @@ Additional context:

- [Discussion](https://github.com/centerofci/mathesar/pull/776#issuecomment-963831424)

## Build components for valid DOM nesting

When working inside a component that _might_ be placed where [phrasing content](https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Content_categories#phrasing_content) is required, be sure to only use phrasing content elements (like `span`) instead of _not_-phrasing content elements (like `div`).

- ✅ Good

`FancyCheckbox.svelte`

```ts
<span class="make-it-fancy">
<input type="checkbox" />
</span>
```

- ❌ Bad because it uses `div` instead of `span`

`FancyCheckbox.svelte`

```ts
<div class="make-it-fancy">
<input type="checkbox" />
</div>
```

Rationale:

- For example, let's build on the "Bad" example above and write the following Svelte code:

```svelte
<label>
<FancyCheckbox />
Foo
</label>
```

That Svelte code will render:

```html
<label>
<div class="make-it-fancy">
<input type="checkbox" />
</div>
Foo
</label>
```

That markup has invalid DOM nesting because a `label` element can only contain phrasing content -- but a `div` is _not_ phrasing content.

Tips:

- You can make a `span` _act_ like a `div` by setting `display: block` if needed.

Notes:

- Not all of our components adhere to this guideline yet.


## `type` vs `interface`

- Prefer `interface` when possible.
- Use `type` when necessary.

## Usage of `{...$$restProps}`
### Usage of `{...$$restProps}`

We use `{...$$restProps}` occasionally in our code despite some drawbacks.

Expand Down

0 comments on commit a6a0b92

Please sign in to comment.