Skip to content

Commit

Permalink
refactor: for route state prefer events over v-model (#3416)
Browse files Browse the repository at this point in the history
In #1966 a watcher has been added to sync boolean query search params.
This watcher caused some issues in nested routes due to a race condition
as described in #3409.

The reason for the watcher (from #1966)
>Whilst I'd prefer not use two way binding models we have some
components that we use that uses these as required values, such as
KInputSwitch

Turns out that the components `KSelect` and `KCheckBox` now support the
`@change` events. When used alongside the `:model-value` we have our
getter and setter separated. This means that we can remove the
additional watcher.

From #3409 
> 4. I had to keep the watcher with the booleans only for the case of
KInputSwitch.

---------

Signed-off-by: schogges <[email protected]>
  • Loading branch information
schogges authored Jan 22, 2025
1 parent 39e150e commit fd4238e
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,6 @@ Defining these "dependencies" here has several advantages:
4. Boolean route params are transformed correctly for the URL. In the above
example the resulting query parameter will be the existence of `?checked`
not `?checked=true` and `?checked=false`
5. Route params can be used as `v-model` values. Ideally we usually use events
to change route properties, but for components that only support `v-model`
you can use the route.param as the v-model. For example `<KCheckBox
v-model="route.params.checked" />` will automatically sync the checkbox state
to the `?checked` query parameter. This aims to make it easier to maintain
state in the browser URL and harder not to

## Setting the `<title>` of the page

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,18 +174,6 @@ const routeView = {
}
const routeParams = reactive<Params>(structuredClone(props.params) as Params)
// Updates the URL for route params if used as a modelValue (for boolean props only)
watch(routeParams, (val) => {
if (route.name === props.name) {
const booleans = Object.fromEntries(Object.entries(val).filter(([key, _value]) => {
return typeof props.params[key] === 'boolean'
}))
if (Object.keys(booleans).length > 0) {
routeUpdate(booleans)
}
}
})
// when any URL params change, normalize/validate/default and reset our actual application params
const redirected = ref<boolean>(false)
watch(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@
>
<template #primary-actions>
<XCheckbox
v-model="route.params.includeEds"
:checked="route.params.includeEds"
:label="t('connections.include_endpoints')"
@change="(value) => route.update({ includeEds: value})"
/>
<XAction
action="refresh"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@
>
<template #primary-actions>
<XCheckbox
v-model="route.params.includeEds"
:checked="route.params.includeEds"
label="Include Endpoints"
@change="(value) => route.update({ includeEds: value})"
/>
<XAction
action="refresh"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,9 @@
#actions
>
<XInputSwitch
v-model="route.params.inactive"
:checked="route.params.inactive"
data-testid="dataplane-outbounds-inactive-toggle"
@change="(value) => route.update({ inactive: value})"
>
<template #label>
Show inactive
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# x-checkbox
23 changes: 23 additions & 0 deletions packages/kuma-gui/src/app/x/components/x-checkbox/XCheckbox.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<template>
<KCheckbox
:model-value="props.checked"
>
<template
v-for="(_, slotName) in slots"
:key="slotName"
#[slotName]="slotProps"
>
<slot
:name="slotName"
v-bind="(slotProps)"
/>
</template>
</KCheckbox>
</template>

<script setup lang="ts">
import { KCheckbox } from '@kong/kongponents'
const props = defineProps<{ checked?: boolean }>()
const slots = defineSlots()
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# x-input-switch
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<template>
<KInputSwitch
:model-value="props.checked"
>
<template
v-for="(_, slotName) in slots"
:key="slotName"
#[slotName]="slotProps"
>
<slot
:name="slotName"
v-bind="(slotProps)"
/>
</template>
</KInputSwitch>
</template>

<script setup lang="ts">
import { KInputSwitch } from '@kong/kongponents'
const props = defineProps<{ checked?: boolean }>()
const slots = defineSlots()
</script>
12 changes: 7 additions & 5 deletions packages/kuma-gui/src/app/x/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Kongponents, { KTooltip, KCard, KPop, KInputSwitch, KCheckbox, KRadio } from '@kong/kongponents'
import Kongponents, { KTooltip, KCard, KPop, KRadio } from '@kong/kongponents'

import XAboutCard from './components/x-about-card/XAboutCard.vue'
import XAction from './components/x-action/XAction.vue'
Expand All @@ -7,13 +7,15 @@ import XAlert from './components/x-alert/XAlert.vue'
import XAnonymous from './components/x-anonymous/XAnonymous.vue'
import XBadge from './components/x-badge/XBadge.vue'
import XBreadcrumbs from './components/x-breadcrumbs/XBreadcrumbs.vue'
import XCheckBox from './components/x-checkbox/XCheckbox.vue'
import XCodeBlock from './components/x-code-block/XCodeBlock.vue'
import XCopyButton from './components/x-copy-button/XCopyButton.vue'
import XDisclosure from './components/x-disclosure/XDisclosure.vue'
import XEmptyState from './components/x-empty-state/XEmptyState.vue'
import XI18n from './components/x-i18n/XI18n.vue'
import XIcon from './components/x-icon/XIcon.vue'
import XInput from './components/x-input/XInput.vue'
import XInputSwitch from './components/x-input-switch/XInputSwitch.vue'
import XLayout from './components/x-layout/XLayout.vue'
import XModal from './components/x-modal/XModal.vue'
import XProgress from './components/x-progress/XProgress.vue'
Expand All @@ -33,8 +35,6 @@ declare module 'vue' {
export interface GlobalComponents {
XCard: typeof KCard
XPop: typeof KPop
XInputSwitch: typeof KInputSwitch
XCheckbox: typeof KCheckbox
XRadio: typeof KRadio
XTooltip: typeof KTooltip
//
Expand All @@ -61,6 +61,8 @@ declare module 'vue' {
XTeleportSlot: typeof XTeleportSlot
XDisclosure: typeof XDisclosure
XAboutCard: typeof XAboutCard
XInputSwitch: typeof XInputSwitch
XCheckbox: typeof XCheckBox
}
}

Expand All @@ -86,8 +88,6 @@ export const services = (app: Record<string, Token>): ServiceDefinition[] => {
['XAlert', XAlert],
['XCard', KCard],
['XPop', KPop],
['XInputSwitch', KInputSwitch],
['XCheckbox', KCheckbox],
['XRadio', KRadio],
['XTooltip', KTooltip],
//
Expand All @@ -113,6 +113,8 @@ export const services = (app: Record<string, Token>): ServiceDefinition[] => {
['XTeleportSlot', XTeleportSlot],
['XDisclosure', XDisclosure],
['XAboutCard', XAboutCard],
['XInputSwitch', XInputSwitch],
['XCheckbox', XCheckBox],
]
},
labels: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@
#actions
>
<XInputSwitch
v-model="route.params.inactive"
:checked="route.params.inactive"
data-testid="dataplane-outbounds-inactive-toggle"
@change="(value) => route.update({ inactive: value})"
>
<template #label>
Show inactive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@
>
<template #primary-actions>
<XCheckbox
v-model="route.params.includeEds"
:checked="route.params.includeEds"
label="Include Endpoints"
@change="(value) => route.update({ includeEds: value})"
/>
<XAction
action="refresh"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,9 @@
#actions
>
<XInputSwitch
v-model="route.params.inactive"
:checked="route.params.inactive"
data-testid="dataplane-outbounds-inactive-toggle"
@change="(value) => route.update({ inactive: value})"
>
<template #label>
Show inactive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@
>
<template #primary-actions>
<XCheckbox
v-model="route.params.includeEds"
:checked="route.params.includeEds"
label="Include Endpoints"
@change="(value) => route.update({ includeEds: value})"
/>
<XAction
action="refresh"
Expand Down

0 comments on commit fd4238e

Please sign in to comment.