Skip to content

Commit

Permalink
refactor(meshes): refactor meshes to use our newer patterns (#3233)
Browse files Browse the repository at this point in the history 
This PR is a little mix of things unfortunately, they are small things
related to `meshes` refactoring, so happy to leave as is but would be
fine to split smaller if thats preferred. The GH "Hide Whitespace"
option is probably good for review.

---

### Collection/Table Loading:

We have this little 'wart' in quite a few places:


https://github.com/kumahq/kuma-gui/blob/765d9f5f2ca3fdc44b5998f946064ed9aa725c7b/packages/kuma-gui/src/app/meshes/views/MeshListView.vue#L36

Not only this but we have a lot of weirdness to support the fact that
`KTable` is the only component we use that has its own loader embedded
in it, for absolutely everything else we use DataLoader to provide
loading states and loading visuals.

I gave DataLoader a new `variant="default | list"` attribute, which is
shorthand for avoiding providing a full `#loading` slot to dataloader.
By specifying `list` you can tell it to use the exact same loader that
KTable uses (KTable just uses `<KSkeleton type="table" />`) so
essentially `<DataLoader variant="list" />` we replicate the exact same
look=and-feel of KTable's loader, but we can also use it for other list
type structures (that aren't KTables) if we need to.

This also means that our listing pages work exactly the same as all our
other pages. No weird `<template #loadable/>` things just to support the
embedded loader inside KTable, and the 'wart' is gone 🎉

I implemented this only in MeshList for the moment, with the plan to
rollout in very near-future PRs.

This led me to realising I could upgrade a few more things in our
`meshes/` module, so I did that while I was here:

### Other Upgrades

- `<div class="stack" />` to `<XLayout type="stack" />`
- `<KCard />` to `<XCard />`
- I removed some unnecessary `div`s entirely.
- I noticed we had `<div class="columns" />` which led me to do the next
section:

### `<XLayout type="columns" />`

Still only 99% sure about XLayout, but I still prefer using it to `<div
class="thing">`. I made a new layout type of `columns` and switched
meshes to use that instead of the div. Its uses the exact same CSS.

---

Lastly I wanted to point out that I really like the name `:variant` for
an attribute that means "I choose this variable property". I've used
`type=""` in the past but found that in Vue this can clash with HTML
`type=""` and I fel back to use `:action` which only worked as a good
name for the specific usecase (I needed it for XAction). `:variant` as a
name would work in every case and would not clash with HTML at all.
Nothing to do as yet, but as a nitty refactor PR at some point we might
want to standardize on `:variant` as a word and then make its usage
consistent on all our components that use things like `:type` or
`:action`.

Signed-off-by: John Cowen <[email protected]>
  • Loading branch information
@johncowen
johncowen authored Dec 3, 2024
1 parent 056fa72 commit 73c3d1a
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 87 deletions.
4 changes: 2 additions & 2 deletions packages/kuma-gui/features/application/Loading.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ Feature: application / loading

Background:
Given the CSS selectors
| Alias | Selector |
| collection-loading | [data-testid$='-collection'] [data-testid='table-skeleton'] |
| Alias | Selector |
| collection-loading | [data-testid='list-skeleton'] |

Scenario: Collections show a loading view
Given the environment
Expand Down
16 changes: 15 additions & 1 deletion packages/kuma-gui/src/app/application/components/app-collection/AppCollection.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@

<script lang="ts" setup generic="Row extends {}">
import { KTable } from '@kong/kongponents'
import { useSlots, ref, watch, Ref } from 'vue'
import { useSlots, ref, watch, Ref, inject } from 'vue'
import { runInDebug } from '../../'
import type { TableHeader as KTableHeader, TablePreferences } from '@kong/kongponents'
type CellAttrParams = {
headerKey: string
Expand All @@ -59,6 +60,19 @@ type ResizeValue = {
type TableHeader = KTableHeader & {
width?: number
}
// when we are inside of a DataLoader make sure its using the `variant="list"`
// but only error in dev mode, if this fails in production we don't want things
// to blow up
const dataLoader = inject<{ props: { variant: string } } | undefined>('data-loader')
if (typeof dataLoader !== 'undefined') {
if (dataLoader.props.variant !== 'list') {
runInDebug(() => {
// throw new Error('Please use <DataLoader variant="list" />')
})
}
}
//
const props = withDefaults(defineProps<{
isSelectedRow?: ((row: Row) => boolean)
items: Row[] | undefined
Expand Down
16 changes: 14 additions & 2 deletions packages/kuma-gui/src/app/application/components/data-source/DataLoader.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,15 @@
:refresh="props.src !== '' ? refresh : () => {}"
>
<LoadingBlock
v-if="props.variant === 'default'"
v-bind="$attrs"
/>
<KSkeleton
v-else
data-testid="list-skeleton"
v-bind="$attrs"
type="table"
/>
</slot>
<slot
v-else
Expand All @@ -84,22 +91,27 @@
typeOf(): any
}" setup
>
import { computed, ref, useSlots } from 'vue'
import { computed, ref, useSlots, provide } from 'vue'
import type { TypeOf } from '@/app/application'
import ErrorBlock from '@/app/common/ErrorBlock.vue'
import LoadingBlock from '@/app/common/LoadingBlock.vue'
const props = withDefaults(defineProps<{
data?: unknown[]
errors?: (Error | undefined)[]
src?: T
loader?: boolean
variant?: 'default' | 'list'
}>(), {
errors: () => [],
data: () => [],
src: '' as any,
loader: true,
variant: 'default',
})
provide('data-loader', {
props,
})
const slots = useSlots()
Expand Down
26 changes: 17 additions & 9 deletions packages/kuma-gui/src/app/meshes/views/MeshDetailView.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
]"
:key="missingTLSPolicy"
>
<AppView :docs="t('meshes.href.docs')">
<AppView
:docs="t('meshes.href.docs')"
>
<template
v-if="!props.mesh.mtlsBackend || missingTLSPolicy"
#notifications
Expand All @@ -50,7 +52,9 @@
:start="t('common.formats.datetime', { value: Date.parse(props.mesh.creationTime) })"
:end="t('common.formats.datetime', { value: Date.parse(props.mesh.modificationTime) })"
/>
<div class="columns">
<XLayout
type="columns"
>
<template
v-for="policy in ['MeshTrafficPermission', 'MeshMetric', 'MeshAccessLog', 'MeshTrace']"
:key="policy"
Expand Down Expand Up @@ -84,13 +88,17 @@
</DefinitionCard>
</template>
</template>
</div>
</XLayout>
</XLayout>
</XCard>

<KCard>
<div class="stack">
<div class="columns">
<XCard>
<XLayout
type="stack"
>
<XLayout
type="columns"
>
<ResourceStatus
:total="data?.services.total ?? 0"
data-testid="services-status"
Expand Down Expand Up @@ -136,9 +144,9 @@
</template>
</template>
</DefinitionCard>
</div>
</div>
</KCard>
</XLayout>
</XLayout>
</XCard>
<ResourceCodeBlock
:resource="mesh.config"
v-slot="{ copy, copying }"
Expand Down
152 changes: 80 additions & 72 deletions packages/kuma-gui/src/app/meshes/views/MeshListView.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,84 +19,92 @@
</h1>
</template>

<div class="stack">
<div v-html="t('meshes.routes.items.intro', {}, { defaultMessage: '' })" />
<KCard>
<DataLoader
:src="uri(sources, `/mesh-insights`, {}, {
page: route.params.page,
size: route.params.size,
})"
<div
v-html="t('meshes.routes.items.intro', {}, { defaultMessage: '' })"
/>

<XCard>
<DataLoader
variant="list"
:src="uri(sources, `/mesh-insights`, {}, {
page: route.params.page,
size: route.params.size,
})"
v-slot="{ data }"
>
<DataCollection
type="meshes"
:items="data.items"
:page="route.params.page"
:page-size="route.params.size"
:total="data.total"
@change="route.update"
v-slot="{ items }"
>
<template
#loadable="{ data }"
<AppCollection
class="mesh-collection"
data-testid="mesh-collection"
:headers="[
{ ...me.get('headers.name'), label: t('meshes.common.name'), key: 'name' },
{ ...me.get('headers.services'), label: t('meshes.routes.items.collection.services'), key: 'services'},
{ ...me.get('headers.dataplanes'), label: t('meshes.routes.items.collection.dataplanes'), key: 'dataplanes'},
{ ...me.get('headers.actions'), label: 'Actions', key: 'actions', hideLabel: true },
]"
:items="items"
:is-selected-row="(row) => row.name === route.params.mesh"
@resize="me.set"
>
<DataCollection
type="meshes"
:items="data?.items ?? [undefined]"
:page="route.params.page"
:page-size="route.params.size"
:total="data?.total"
@change="route.update"
<template
#name="{ row: item }"
>
<AppCollection
class="mesh-collection"
data-testid="mesh-collection"
:headers="[
{ ...me.get('headers.name'), label: t('meshes.common.name'), key: 'name' },
{ ...me.get('headers.services'), label: t('meshes.routes.items.collection.services'), key: 'services'},
{ ...me.get('headers.dataplanes'), label: t('meshes.routes.items.collection.dataplanes'), key: 'dataplanes'},
{ ...me.get('headers.actions'), label: 'Actions', key: 'actions', hideLabel: true },
]"
:items="data?.items"
:is-selected-row="(row) => row.name === route.params.mesh"
@resize="me.set"
<XAction
data-action
:to="{
name: 'mesh-detail-view',
params: {
mesh: item.name,
},
query: {
page: route.params.page,
size: route.params.size,
},
}"
>
<template #name="{ row: item }">
<XAction
data-action
:to="{
name: 'mesh-detail-view',
params: {
mesh: item.name,
},
query: {
page: route.params.page,
size: route.params.size,
},
}"
>
{{ item.name }}
</XAction>
</template>
{{ item.name }}
</XAction>
</template>

<template #services="{ row: item }">
{{ item.services.internal }}
</template>
<template
#services="{ row: item }"
>
{{ item.services.internal }}
</template>

<template #dataplanes="{ row: item }">
{{ item.dataplanesByType.standard.online }} / {{ item.dataplanesByType.standard.total }}
</template>
<template #actions="{ row: item }">
<XActionGroup>
<XAction
:to="{
name: 'mesh-detail-view',
params: {
mesh: item.name,
},
}"
>
{{ t('common.collection.actions.view') }}
</XAction>
</XActionGroup>
</template>
</AppCollection>
</DataCollection>
</template>
</DataLoader>
</KCard>
</div>
<template
#dataplanes="{ row: item }"
>
{{ item.dataplanesByType.standard.online }} / {{ item.dataplanesByType.standard.total }}
</template>
<template
#actions="{ row: item }"
>
<XActionGroup>
<XAction
:to="{
name: 'mesh-detail-view',
params: {
mesh: item.name,
},
}"
>
{{ t('common.collection.actions.view') }}
</XAction>
</XActionGroup>
</template>
</AppCollection>
</DataCollection>
</DataLoader>
</XCard>
</AppView>
</RouteView>
</template>
Expand Down
16 changes: 15 additions & 1 deletion packages/kuma-gui/src/app/x/components/x-layout/XLayout.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</template>
<script lang="ts" setup>
const props = withDefaults(defineProps<{
type?: 'stack' | 'separated'
type?: 'stack' | 'separated' | 'columns'
size?: 'small' | 'normal'
}>(), {
type: 'stack',
Expand All @@ -26,4 +26,18 @@ const props = withDefaults(defineProps<{
flex-wrap: wrap;
gap: $kui-space-40;
}
.columns {
--threshold: 40rem;
display: flex;
flex-wrap: wrap;
gap: $kui-space-80;
}
.columns > * {
flex-grow: 1;
flex-basis: calc((var(--threshold) - 100%) * 999);
min-inline-size: 0;
}
</style>

0 comments on commit 73c3d1a

Please sign in to comment.