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

Jump to bottom

Extend Selection Menu to allow text selection #11352

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
+126 −14
Open

Extend Selection Menu to allow text selection #11352

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c78e79d
component working, changes not propagating
marthasharkey Oct 17, 2024
e4de551
Change heading and tooltip
marthasharkey Oct 17, 2024
64f68a0
remove log
marthasharkey Oct 17, 2024
9bc004c
make seperate component
marthasharkey Oct 18, 2024
81399ae
revert selection dropdown changes
marthasharkey Oct 18, 2024
eee5725
revert selection dropdown changes
marthasharkey Oct 18, 2024
95b51c2
sort types and extend to toolbar
marthasharkey Oct 18, 2024
b789707
Merge branch 'develop' into wip/mk/text-drop-down-component-y-axis
marthasharkey Oct 18, 2024
b20f534
Merge branch 'develop' into wip/mk/text-drop-down-component-y-axis
marthasharkey Oct 21, 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
2 changes: 2 additions & 0 deletions app/gui/src/project-view/components/GraphEditor/GraphVisualization/VisualizationToolbar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ useEvent(window, 'pointerup', (e) => interaction.handlePointerEvent(e, 'pointeru
v-model="item.selected.value"
:options="item.options"
:title="item.title"
:heading="item.heading ?? null"
:isTextDropdown="item.isTextDropdown ?? false"
alwaysShowArrow
/>
<div v-else>?</div>
Expand Down
28 changes: 21 additions & 7 deletions app/gui/src/project-view/components/SelectionDropdown.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const _props = defineProps<{
title?: string | undefined
labelButton?: boolean
alwaysShowArrow?: boolean
isTextDropdown?: boolean
heading?: string | null
Copy link
Contributor

Choose a reason for hiding this comment

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

Not-present (undefined) or null would have the same meaning--let's avoid a distinction without a difference.

Suggested change
heading?: string | null
heading?: string | undefined

}>()

const open = ref(false)
Expand All @@ -23,12 +25,22 @@ const open = ref(false)
<DropdownMenu v-model:open="open" :title="title" :alwaysShowArrow="alwaysShowArrow">
<template #button>
<template v-if="options[selected]">
<SvgIcon :name="options[selected]!.icon" :style="options[selected]!.iconStyle" />
<div
v-if="labelButton && options[selected]!.label"
class="iconLabel"
v-text="options[selected]!.label"
/>
<template v-if="isTextDropdown">
<div v-if="heading" v-text="heading" />
<div
v-if="options[selected]?.label"
class="iconLabel"
v-text="options[selected]?.label"
/>
</template>
<template v-else-if="options[selected]!.icon">
<SvgIcon :name="options[selected]!.icon!" :style="options[selected]!.iconStyle" />
<div
v-if="labelButton && options[selected]!.label"
class="iconLabel"
v-text="options[selected]!.label"
/>
</template>
</template>
</template>
<template #entries>
Expand All @@ -40,7 +52,9 @@ const open = ref(false)
@update:modelValue="$event && (selected = key)"
@click="open = false"
>
<SvgIcon :name="option.icon" :style="option.iconStyle" :data-testid="option.dataTestid" />
<template v-if="option.icon">
<SvgIcon :name="option.icon" :style="option.iconStyle" :data-testid="option.dataTestid" />
</template>
<div v-if="option.label" class="iconLabel" v-text="option.label" />
</MenuButton>
</template>
Expand Down
56 changes: 44 additions & 12 deletions app/gui/src/project-view/components/visualizations/ScatterplotVisualization.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ const yLabelLeft = computed(
getTextWidthBySizeAndFamily(data.value.axis.y.label, LABEL_FONT_STYLE) / 2,
)
const yLabelTop = computed(() => -margin.value.left + 15)
const showYLabelText = computed(() => !data.value.is_multi_series)
const xTickFormat = computed(() => {
switch (data.value.x_value_type) {
case 'Time':
Expand All @@ -308,6 +307,22 @@ const xTickFormat = computed(() => {
return '%d/%m/%Y %H:%M:%S'
}
})
const seriesLabels = computed(() =>
Object.keys(data.value.axis)
.filter((s) => s != 'x')
.map((s) => {
return data.value.axis[s as keyof AxesConfiguration].label
}),
Comment on lines +320 to +324
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are using both the keys and the values of data.value.axis, using Object.entries might be simpler, and would avoid the type cast.

)

const yLabelText = computed(() => {
if (!data.value.is_multi_series) {
return data.value.axis.y.label
}
if (yAxisSelected.value) {
return yAxisSelected.value === 'none' ? null : yAxisSelected.value
} else return null
})
const isUsingIndexForX = computed(() => data.value.axis.x.label === 'index')

watchEffect(() => {
Expand Down Expand Up @@ -722,26 +737,21 @@ watchPostEffect(() => {

watchPostEffect(() => {
if (data.value.is_multi_series) {
const seriesLabels = Object.keys(data.value.axis)
.filter((s) => s != 'x')
.map((s) => {
return data.value.axis[s as keyof AxesConfiguration].label
})
const formatLabel = (string: string) =>
string.length > 10 ? `${string.substr(0, 10)}...` : string

const color = d3
.scaleOrdinal<string>()
.domain(seriesLabels)
.domain(seriesLabels.value)
.range(d3.schemeCategory10)
.domain(seriesLabels)
.domain(seriesLabels.value)

d3Legend.value.selectAll('circle').remove()
d3Legend.value.selectAll('text').remove()

d3Legend.value
.selectAll('dots')
.data(seriesLabels)
.data(seriesLabels.value)
.enter()
.append('circle')
.attr('cx', function (d, i) {
Expand All @@ -753,7 +763,7 @@ watchPostEffect(() => {

d3Legend.value
.selectAll('labels')
.data(seriesLabels)
.data(seriesLabels.value)
.enter()
.append('text')
.attr('x', function (d, i) {
Expand Down Expand Up @@ -834,6 +844,17 @@ function zoomToSelected(override?: boolean) {

useEvent(document, 'keydown', bindings.handler({ zoomToSelected: () => zoomToSelected() }))

const yAxisSelected = ref('none')
const makeSeriesLabelOptions = () => {
const seriesOptions: { [key: string]: { label: string } } = {}
seriesLabels.value.forEach((label, index) => {
seriesOptions[label] = {
label: label,
}
})
return seriesOptions
}

config.setToolbar([
{
icon: 'select',
Expand All @@ -857,6 +878,18 @@ config.setToolbar([
disabled: () => !createNewFilterNodeEnabled.value,
onClick: createNewFilterNode,
},
{
selected: yAxisSelected,
title: 'Choose Y Axis Label',
isTextDropdown: true,
heading: 'Y Axis Label: ',
options: {
none: {
label: 'No Label',
},
...makeSeriesLabelOptions(),
},
},
])
</script>

Expand All @@ -881,12 +914,11 @@ config.setToolbar([
v-text="data.axis.x.label"
></text>
<text
v-if="showYLabelText"
class="label label-y"
text-anchor="end"
:x="yLabelLeft"
:y="yLabelTop"
v-text="data.axis.y.label"
v-text="yLabelText"
></text>
<g ref="pointsNode" clip-path="url(#clip)"></g>
<g ref="zoomNode" class="zoom" :width="boxWidth" :height="boxHeight" fill="none">
Expand Down
6 changes: 5 additions & 1 deletion app/gui/src/project-view/components/visualizations/toolbar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { ToValue } from '@/util/reactivity'
import { Ref } from 'vue'

export interface Button {
icon: Icon | URLString
iconStyle?: Record<string, string>
title?: string
dataTestid?: string
Expand All @@ -13,21 +12,26 @@ export interface Button {
export interface ActionButton extends Button {
onClick: () => void
disabled?: ToValue<boolean>
icon: Icon | URLString
}

export interface ToggleButton extends Button {
toggle: Ref<boolean>
disabled?: ToValue<boolean>
icon: Icon | URLString
}

export interface SelectionMenuOption extends Button {
label?: string
icon?: Icon | URLString
}

export interface SelectionMenu {
selected: Ref<string>
title?: string
options: Record<string, SelectionMenuOption>
isTextDropdown?: boolean
heading?: string
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of the isTextDropdown field, let's introduce a separate type:

export interface TextSelectionMenu {
  type: 'textSelectionMenu'
  selected: Ref<string>
  title?: string
  options: Record<string, TextSelectionMenuOption>
  heading?: string
}

The reason for the type field approach is probably not apparent--it fits into my plan for the ToolbarItem types (as we add more and their definitions get more complex):

Eventually, every type will have a type field that must contain a specific value if present; however, the field will be optional for any type that can be recognized by its fields, to allow concise toolbar definitions.

So far, every type has had distinctive enough fields that we haven't needed to make any type fields required (and I haven't got around to defining optional type fields). The new TextSelectionMenu has similar fields to the normal SelectionMenu, so its type field is not optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok! Thanks, I wasn't sure if it was best to adapt the current dropdown or make a new one so I'll separate it now!


export type ToolbarItem = ActionButton | ToggleButton | SelectionMenu
Expand Down
Loading