Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(dashboards and charts): editable tiles | slottable menu items #1758

Merged
merged 4 commits into from
Nov 4, 2024
Merged

feat(dashboards and charts): editable tiles | slottable menu items #1758

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b103fc2
feat(dashboards and charts): editable tiles | slottable menu items
filipgutica Nov 1, 2024
231a753
fix: move editable to context
filipgutica Nov 1, 2024
24443a5
fix: address comment
filipgutica Nov 1, 2024
fd442ec
fix: address comment
filipgutica Nov 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/analytics/analytics-chart/src/components/AnalyticsChart.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
{{ i18n.t('csvExport.exportButton') }}
</span>
</KDropdownItem>
<slot name="menu-items" />
</template>
</KDropdown>
<!-- Keep outside of dropdown, so we can independently affect its visibility -->
Expand Down Expand Up @@ -230,6 +231,11 @@ const props = defineProps({
required: false,
default: false,
},
showMenu: {
type: Boolean,
required: false,
default: false,
},
})

const emit = defineEmits<{
Expand Down Expand Up @@ -361,7 +367,7 @@ const showChartHeader = computed(() => {
return (hasValidChartData.value && resultSetTruncated.value && maxEntitiesShown.value) || props.chartTitle || (props.allowCsvExport && hasValidChartData.value)
})

const hasMenuOptions = computed(() => (props.allowCsvExport && hasValidChartData.value) || !!props.goToExplore)
const hasMenuOptions = computed(() => (props.allowCsvExport && hasValidChartData.value) || !!props.goToExplore || props.showMenu)

const timeSeriesGranularity = computed<GranularityValues>(() => {

Expand Down
8 changes: 7 additions & 1 deletion packages/analytics/dashboard-renderer/sandbox/pages/RendererDemo.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
:class="{ 'custom-styling': isToggled}"
:config="(dashboardConfig as DashboardConfig)"
:context="context"
@edit-tile="onEditTile"
>
<template #slot-1>
<div class="slot-container">
Expand All @@ -32,7 +33,7 @@
</template>

<script setup lang="ts">
import type { DashboardConfig, DashboardRendererContext, TileConfig } from '../../src'
import type { DashboardConfig, DashboardRendererContext, TileConfig, TileDefinition, GridTile } from '../../src'
import { DashboardRenderer } from '../../src'
import { inject, ref } from 'vue'
import { ChartMetricDisplay } from '@kong-ui-public/analytics-chart'
Expand All @@ -45,6 +46,7 @@ const appLinks: SandboxNavigationItem[] = inject('app-links', [])
const context: DashboardRendererContext = {
filters: [],
refreshInterval: 0,
editable: true,
}

const dashboardConfig: DashboardConfig = {
Expand Down Expand Up @@ -239,6 +241,10 @@ const dashboardConfig: DashboardConfig = {

const isToggled = ref(false)

const onEditTile = (tile: GridTile<TileDefinition>) => {
console.log('@edit-tile', tile)
}

</script>

<style lang="scss" scoped>
Expand Down
43 changes: 10 additions & 33 deletions packages/analytics/dashboard-renderer/src/components/BarChartRenderer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,19 @@
<!-- BarChartRenderer.vue -->
<template>
<QueryDataProvider
v-slot="{ data }"
<BaseAnalyticsChartRenderer
:chart-options="chartOptions"
:context="context"
:editable="context.editable"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're already passing in context -- no need for the extra prop. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

:extra-props="{ showAnnotations: false }"
:height="height"
:query="query"
:query-ready="queryReady"
>
<div class="analytics-chart">
<AnalyticsChart
:allow-csv-export="chartOptions.allowCsvExport"
:chart-data="data"
:chart-options="options"
:chart-title="chartOptions.chartTitle"
legend-position="bottom"
:show-annotations="false"
:synthetics-data-key="chartOptions.syntheticsDataKey"
tooltip-title=""
/>
</div>
</QueryDataProvider>
/>
</template>

<script setup lang="ts">
import BaseAnalyticsChartRenderer from './BaseAnalyticsChartRenderer.vue'
import type { BarChartOptions, RendererProps } from '../types'
import QueryDataProvider from './QueryDataProvider.vue'
import { computed } from 'vue'
import type { AnalyticsChartOptions } from '@kong-ui-public/analytics-chart'
import { AnalyticsChart } from '@kong-ui-public/analytics-chart'

const props = defineProps<RendererProps<BarChartOptions>>()

const options = computed((): AnalyticsChartOptions => ({
type: props.chartOptions.type,
stacked: props.chartOptions.stacked,
chartDatasetColors: props.chartOptions.chartDatasetColors,
}))
defineProps<RendererProps<BarChartOptions>>()
</script>

<style scoped lang="scss">
.analytics-chart {
height: v-bind('`${height}px`');
}
</style>
62 changes: 62 additions & 0 deletions packages/analytics/dashboard-renderer/src/components/BaseAnalyticsChartRenderer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<template>
<QueryDataProvider
v-slot="{ data }"
:context="context"
:query="query"
:query-ready="queryReady"
>
<div class="analytics-chart">
<AnalyticsChart
:allow-csv-export="chartOptions.allowCsvExport"
:chart-data="data"
:chart-options="options"
:chart-title="chartOptions.chartTitle"
legend-position="bottom"
:show-menu="context.editable"
:synthetics-data-key="chartOptions.syntheticsDataKey"
tooltip-title=""
v-bind="extraProps"
>
<template
v-if="context.editable"
#menu-items
>
<KDropdownItem @click="editTile">
{{ i18n.t('renderer.edit') }}
</KDropdownItem>
</template>
</AnalyticsChart>
</div>
</QueryDataProvider>
</template>

<script setup lang="ts">
import type { RendererProps } from '../types'
import QueryDataProvider from './QueryDataProvider.vue'
import { computed, defineProps } from 'vue'
import type { AnalyticsChartOptions } from '@kong-ui-public/analytics-chart'
import { AnalyticsChart } from '@kong-ui-public/analytics-chart'
import composables from '../composables'

const props = defineProps<RendererProps<any> & { extraProps?: Record<string, any> }>()
const emit = defineEmits<{
(e: 'edit-tile'): void
}>()
const { i18n } = composables.useI18n()

const options = computed((): AnalyticsChartOptions => ({
type: props.chartOptions.type,
stacked: props.chartOptions.stacked ?? false,
chartDatasetColors: props.chartOptions.chartDatasetColors,
}))

const editTile = () => {
emit('edit-tile')
}
</script>

<style scoped lang="scss">
.analytics-chart {
height: v-bind('`${height}px`');
}
</style>
16 changes: 15 additions & 1 deletion packages/analytics/dashboard-renderer/src/components/DashboardRenderer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
:definition="tile.meta"
:height="tile.layout.size.rows * (config.tileHeight || DEFAULT_TILE_HEIGHT) + parseInt(KUI_SPACE_70, 10)"
:query-ready="queryReady"
@edit-tile="onEditTile(tile)"
/>
</template>
</GridLayout>
Expand Down Expand Up @@ -53,6 +54,10 @@ const props = defineProps<{
config: DashboardConfig,
}>()

const emit = defineEmits<{
(e: 'edit-tile', tile: GridTile<TileDefinition>): void
}>()

const { i18n } = composables.useI18n()

// Note: queryBridge is not directly used by the DashboardRenderer component. It is required by many of the
Expand Down Expand Up @@ -129,7 +134,7 @@ const gridTiles = computed(() => {
})

const mergedContext = computed<DashboardRendererContextInternal>(() => {
let { tz, refreshInterval } = props.context
let { tz, refreshInterval, editable } = props.context

if (!tz) {
tz = (new Intl.DateTimeFormat()).resolvedOptions().timeZone
Expand All @@ -140,17 +145,26 @@ const mergedContext = computed<DashboardRendererContextInternal>(() => {
refreshInterval = DEFAULT_TILE_REFRESH_INTERVAL_MS
}

if (editable === undefined) {
editable = false
}

return {
...props.context,
tz,
timeSpec: timeSpec.value,
refreshInterval,
editable,
}
})

// Right now, tiles don't have unique keys. Perhaps in the future they will,
// and we can use that instead of `index` as the fragment key.

const onEditTile = (tile: GridTile<TileDefinition>) => {
emit('edit-tile', tile)
}

</script>

<style lang="scss" scoped>
Expand Down
10 changes: 10 additions & 0 deletions packages/analytics/dashboard-renderer/src/components/DashboardTile.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
:is="componentData.component"
v-if="componentData"
v-bind="componentData.rendererProps"
@edit-tile="onEditTile"
/>
</div>
</template>
Expand Down Expand Up @@ -34,6 +35,10 @@ const props = withDefaults(defineProps<{
height: DEFAULT_TILE_HEIGHT,
})

const emit = defineEmits<{
(e: 'edit-tile', tile: TileDefinition): void
}>()

const rendererLookup: Record<DashboardTileType, Component | undefined> = {
'timeseries_line': TimeseriesChartRenderer,
'horizontal_bar': BarChartRenderer,
Expand All @@ -56,9 +61,14 @@ const componentData = computed(() => {
queryReady: props.queryReady,
chartOptions: props.definition.chart,
height: props.height - PADDING_SIZE * 2,
editable: props.context.editable,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to pass in this prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it's still there...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, my bad. I got rid of using the prop, forgot to revert this line 😅
24443a5

},
}
})

const onEditTile = () => {
emit('edit-tile', props.definition)
}
</script>

<style lang="scss" scoped>
Expand Down
30 changes: 18 additions & 12 deletions packages/analytics/dashboard-renderer/src/components/GoldenSignalsRenderer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,24 @@ const overrideTimeframe: Ref<Timeframe> = computed(() => {
return relativePeriod
})

const options = computed<ProviderProps>(() => ({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a type error here that's somehow getting past CI... 🤔

No overload matches this call.
  Overload 1 of 2, '(getter: ComputedGetter<Partial<{ description: string; containerTitle: string; refreshInterval: number; datasource: "advanced" | "basic"; dimension: "api_product" | "api_product_version" | "control_plane" | ... 9 more ... | "iso_code"; ... 8 more ...; percentileLatency: boolean; }> & Omit<...>>, debugOptions?: DebuggerOptions | undefined): ComputedRef<...>', gave the following error.
    Type '"advanced" | "basic" | "ai"' is not assignable to type '"advanced" | "basic" | undefined'.
      Type '"ai"' is not assignable to type '"advanced" | "basic" | undefined'.
  Overload 2 of 2, '(options: WritableComputedOptions<Partial<{ description: string; containerTitle: string; refreshInterval: number; datasource: "advanced" | "basic"; dimension: "api_product" | "api_product_version" | "control_plane" | ... 9 more ... | "iso_code"; ... 8 more ...; percentileLatency: boolean; }> & Omit<...>, Partial<...> & Omit<...>>, debugOptions?: DebuggerOptions | undefined): WritableComputedRef<...>', gave the following error.
    Argument of type '() => { datasource: "advanced" | "basic" | "ai"; overrideTimeframe: Timeframe; tz: string; additionalFilter: ExploreFilter[]; longCardTitles: boolean | undefined; containerTitle: string | undefined; description: string | undefined; percentileLatency: boolean | undefined; refreshInterval: number; queryReady: boolean; }' is not assignable to parameter of type 'WritableComputedOptions<Partial<{ description: string; containerTitle: string; refreshInterval: number; datasource: "advanced" | "basic"; dimension: "api_product" | "api_product_version" | "control_plane" | ... 9 more ... | "iso_code"; ... 8 more ...; percentileLatency: boolean; }> & Omit<...>, Partial<...> & Omit<....'.ts-plugin(2769)
MetricsProvider.vue.d.ts(67, 5): The expected type comes from property 'datasource' which is declared here on type 'Partial<{ description: string; containerTitle: string; refreshInterval: number; datasource: "advanced" | "basic"; dimension: "api_product" | "api_product_version" | "control_plane" | ... 9 more ... | "iso_code"; ... 8 more ...; percentileLatency: boolean; }> & Omit<...>'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's from updates to analytics-utilities which introduced the ai datasource, while the provider props in metrics provider defines datasource property like so: datasource?: "basic" | "advanced" | undefined;

Not sure if my change is the right one, or we should add ai to the acceptable datasources in MetricsProvider

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. I'll make a note to look at this later. I think this is the fault of the schema; it shouldn't allow setting ai as a datasource for metric cards. I think what you have for now is fine.

datasource: props.query?.datasource,
overrideTimeframe: overrideTimeframe.value,
tz: props.context.tz,
additionalFilter: props.context.filters as ExploreFilter[], // TODO: Decide how to handle metric card filters.
longCardTitles: props.chartOptions.longCardTitles,
containerTitle: props.chartOptions.chartTitle,
description: props.chartOptions.description,
percentileLatency: props.chartOptions.percentileLatency,
refreshInterval: props.context.refreshInterval,
queryReady: props.queryReady,
}))
const options = computed<ProviderProps>(() => {
const datasource = props.query?.datasource
if (datasource && datasource !== 'advanced' && datasource !== 'basic') {
throw new Error(`Invalid datasource value: ${datasource}`)
}
return {
datasource: props.query?.datasource,
overrideTimeframe: overrideTimeframe.value,
tz: props.context.tz,
additionalFilter: props.context.filters as ExploreFilter[], // TODO: Decide how to handle metric card filters.
longCardTitles: props.chartOptions.longCardTitles,
containerTitle: props.chartOptions.chartTitle,
description: props.chartOptions.description,
percentileLatency: props.chartOptions.percentileLatency,
refreshInterval: props.context.refreshInterval,
queryReady: props.queryReady,
}
})
</script>

<style scoped lang="scss">
Expand Down
50 changes: 10 additions & 40 deletions packages/analytics/dashboard-renderer/src/components/TimeseriesChartRenderer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,48 +1,18 @@
<!-- TimeseriesChartRenderer.vue -->
<template>
<QueryDataProvider
v-slot="{ data }"
<BaseAnalyticsChartRenderer
:chart-options="chartOptions"
:context="context"
:editable="context.editable"
:height="height"
:query="query"
:query-ready="queryReady"
>
<div class="analytics-chart">
<AnalyticsChart
:allow-csv-export="chartOptions.allowCsvExport"
:chart-data="data"
:chart-options="options"
:chart-title="chartOptions.chartTitle"
legend-position="bottom"
:synthetics-data-key="chartOptions.syntheticsDataKey"
tooltip-title=""
/>
</div>
</QueryDataProvider>
/>
</template>
<script setup lang="ts">
import type { RendererProps, TimeseriesChartOptions } from '../types'
import QueryDataProvider from './QueryDataProvider.vue'
import { computed } from 'vue'
import type { AnalyticsChartOptions } from '@kong-ui-public/analytics-chart'
import { AnalyticsChart } from '@kong-ui-public/analytics-chart'

const props = defineProps<RendererProps<TimeseriesChartOptions>>()

const options = computed((): AnalyticsChartOptions => {
// Default `stacked` to false.
const stacked = props.chartOptions.stacked ?? false
<script setup lang="ts">
import BaseAnalyticsChartRenderer from './BaseAnalyticsChartRenderer.vue'
import type { TimeseriesChartOptions, RendererProps } from '../types'

// Note that `fill` and `stacked` are linked; it's not possible to have a non-filled stacked chart.
// This matches our intuitions about how these charts work.
return {
type: props.chartOptions.type,
stacked,
chartDatasetColors: props.chartOptions.chartDatasetColors,
}
})
defineProps<RendererProps<TimeseriesChartOptions>>()
</script>

<style scoped lang="scss">
.analytics-chart {
height: v-bind('`${height}px`');
}
</style>
3 changes: 2 additions & 1 deletion packages/analytics/dashboard-renderer/src/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"24h": "Last 24-Hour Summary",
"7d": "Last 7-Day Summary",
"30d": "Last 30-Day Summary"
}
},
"edit": "Edit"
},
"queryDataProvider": {
"timeRangeExceeded": "The time range for this report is outside of your organization's data retention period"
Expand Down
Loading
Loading