Skip to content

Commit

Permalink
add more guidelines
Browse files Browse the repository at this point in the history
  • Loading branch information
OlivierFL committed Dec 20, 2024
1 parent b6ebd76 commit 0d40b19
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 10 deletions.
14 changes: 5 additions & 9 deletions @xen-orchestra/web-core/docs/component-variants.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ Then, we can define the props needed to handle the variants:
```ts
// TS types here

const props = defineProps<{
const { variant, accent, size, disabled } = defineProps<{
variant: ButtonVariant
accent: ButtonAccent
size: ButtonSize
Expand All @@ -124,16 +124,12 @@ Given the props we defined above, and to match the class names convention, we ca
A way to do it is as follows:

```ts
const classNames = computed(() => [
toVariants({
variant: props.variant,
accent: props.accent,
size: props.size,
disabled: props.disabled,
}),
])
const classNames = computed(() => toVariants({ variant, accent, size, disabled }))
```

> [!TIP]
> The `toVariants()` helper SHOULD be used in the script section of the component, and not in the template section (see [best practices](./guidelines/best-practices.md#component-should-not-use-functions-in-the-template-to-compute-states-or-values) for more information).
Let's take an example where we want to use the "Button" component with this DS variant:

```
Expand Down
56 changes: 55 additions & 1 deletion @xen-orchestra/web-core/docs/guidelines/best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,66 @@ In this example, the first button will call `deleteMe(event)` and thus will alwa

See also: [inline handlers](https://vuejs.org/guide/essentials/event-handling.html#inline-handlers) in Vue.js documentation.

## Component SHOULD NOT use functions in the template to compute states or values

If using functions in the template to compute states or values, it will be called on every render, which can lead to performance issues.

Use `computed` instead, which will cache the result, and only recompute when the dependencies change.

❌ Bad

```vue
<template>
<UiButton :icon="getIcon(object.connected)" />
</template>
<script setup lang="ts">
function getIcon(connected) {
return connected ? 'check' : 'close'
}
</script>
```

✅ Good

```vue
<template>
<UiButton :icon="objectIcon" />
</template>
<script setup lang="ts">
import { computed } from 'vue'
const objectIcon = computed(() => (object.connected ? 'check' : 'close'))
</script>
```

If you find yourself using this kind of functions in the template, maybe it's a good sign that a refactoring is needed (i.e., extracting the logic in a subcomponent).

There are some exceptions, for example, when using `$t` for translations inside the template.

## Components SHOULD have a Story

> [!TIP]
> For now, stories are stored in
> `@xen-orchestra/lite/src/stories` and can only be written for XO Lite and XO Core components.
## Helpers and utilities SHOULD be used when necessary

Some common utilities are already available in the projects, so be sure to check if there is already a helper or utility that can be used before reinventing the wheel.

You'll find them in these directories:

- Web Core:
- `@xen-orchestra/web-core/src/composables`
- `@xen-orchestra/web-core/src/utils`
- `@xen-orchestra/web-core/src/packages`
- XO 6:
- `@xen-orchestra/web/src/composables`
- `@xen-orchestra/web/src/utils`
- XO Lite:
- `@xen-orchestra/lite/src/composables`

## VueUse SHOULD be used when necessary

Instead of recreating some functionality, [VueUse](https://vueuse.org) provides a lot of utilities for common use cases (e.g., accessing browser APIs, using local storage, etc.).
If you don't find what you need in our helpers, [VueUse](https://vueuse.org) provides a lot of utilities for common use cases (e.g., accessing browser APIs, using local storage, etc.).

0 comments on commit 0d40b19

Please sign in to comment.