Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add three new front-end coding standards #33

Merged
merged 2 commits into from
Feb 4, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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