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 b4f07d6
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 9 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
38 changes: 38 additions & 0 deletions @xen-orchestra/web-core/docs/guidelines/best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,44 @@ 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]
Expand Down

0 comments on commit b4f07d6

Please sign in to comment.