diff --git a/engineering/architecture/front-end-standards.md b/engineering/architecture/front-end-standards.md index 39e84a201..b9dbab815 100644 --- a/engineering/architecture/front-end-standards.md +++ b/engineering/architecture/front-end-standards.md @@ -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: @@ -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 + + + + ``` + +- ❌ Bad because it uses `div` instead of `span` + + `FancyCheckbox.svelte` + + ```ts +
+ +
+ ``` + +Rationale: + +- For example, let's build on the "Bad" example above and write the following Svelte code: + + ```svelte + + ``` + + That Svelte code will render: + + ```html + + ``` + + 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. @@ -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. @@ -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 - - - - ``` - -- ❌ Bad because it uses `div` instead of `span` - - `FancyCheckbox.svelte` - ```ts -
- -
- ``` - -Rationale: - -- For example, let's build on the "Bad" example above and write the following Svelte code: - - ```svelte - - ``` - - That Svelte code will render: - - ```html - - ``` - - 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.