-
Notifications
You must be signed in to change notification settings - Fork 888
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.
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
[Feature anywhere] Add annotation click handler #3777
Changes from all commits
edab515
5fca51a
0fe8e82
325c9fc
09d6cbb
e4921c6
4cfdca4
5912441
76d8744
e469fe9
ed817b2
a53f8ab
618ef27
4669fca
67cc5f5
6d9b9b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
* | ||
* Any modifications Copyright OpenSearch Contributors. See | ||
* GitHub history for details. | ||
*/ | ||
|
||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { i18n } from '@osd/i18n'; | ||
import { Trigger } from '.'; | ||
|
||
export const EXTERNAL_ACTION_TRIGGER = 'EXTERNAL_ACTION_TRIGGER'; | ||
export const externalActionTrigger: Trigger<'EXTERNAL_ACTION_TRIGGER'> = { | ||
id: EXTERNAL_ACTION_TRIGGER, | ||
title: i18n.translate('uiActions.triggers.externalActionTitle', { | ||
defaultMessage: 'Single click', | ||
}), | ||
description: i18n.translate('uiActions.triggers.externalActionDescription', { | ||
defaultMessage: | ||
'A data point click on the visualization used to trigger external action like show flyout, etc.', | ||
}), | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
export enum VisAnnotationType { | ||
POINT_IN_TIME_ANNOTATION = 'POINT_IN_TIME_ANNOTATION', | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ import { euiPaletteColorBlind } from '@elastic/eui'; | |
import { euiThemeVars } from '@osd/ui-shared-deps/theme'; | ||
import { i18n } from '@osd/i18n'; | ||
// @ts-ignore | ||
import { Signal } from 'vega'; | ||
import { vega, vegaLite } from '../lib/vega'; | ||
import { OpenSearchQueryParser } from './opensearch_query_parser'; | ||
import { Utils } from './utils'; | ||
|
@@ -323,6 +324,52 @@ The URL is an identifier only. OpenSearch Dashboards and your browser will never | |
delete this.spec.autosize; | ||
} | ||
} | ||
|
||
if (this._config?.signals) { | ||
Object.entries(this._config?.signals).forEach(([markId, signals]: [string, any]) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to clarify will this break any existing spec that uses signals? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. Since we control this |
||
const mark = this.getMarkWithStyle(this.spec.marks, markId); | ||
|
||
if (mark) { | ||
signals.forEach((signal: Signal) => { | ||
signal.on?.forEach((eventObj) => { | ||
// We are prepending mark name here so that the signals only listens to the events on | ||
// the elements related to this mark | ||
eventObj.events = `@${mark.name}:${eventObj.events}`; | ||
}); | ||
}); | ||
this.spec.signals = (this.spec.signals || []).concat(signals); | ||
} | ||
}); | ||
} | ||
} | ||
|
||
/** | ||
* This method recursively looks for a mark that includes the given style. | ||
* Returns undefined if it doesn't find it. | ||
*/ | ||
getMarkWithStyle(marks: any[], style: string): any { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like some unit tests here would be nice to validate all the conditional branching. |
||
if (!marks) { | ||
return undefined; | ||
} | ||
|
||
if (Array.isArray(marks)) { | ||
const markWithStyle = marks.find((mark) => { | ||
return mark.style?.includes(style); | ||
}); | ||
|
||
if (markWithStyle) { | ||
return markWithStyle; | ||
} | ||
|
||
for (let i = 0; i < marks.length; i++) { | ||
const res = this.getMarkWithStyle(marks[i].marks, style); | ||
if (res) { | ||
return res; | ||
} | ||
} | ||
|
||
return undefined; | ||
} | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,7 @@ export interface ExprVisAPIEvents { | |
filter: (data: any) => void; | ||
brush: (data: any) => void; | ||
applyFilter: (data: any) => void; | ||
externalAction: (data: any) => void; | ||
} | ||
|
||
export interface ExprVisAPI { | ||
|
@@ -99,6 +100,10 @@ export class ExprVis extends EventEmitter { | |
if (!this.eventsSubject) return; | ||
this.eventsSubject.next({ name: 'applyFilter', data }); | ||
}, | ||
externalAction: (data: any) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this planned for functional testing, or can it be unit tested? |
||
if (!this.eventsSubject) return; | ||
this.eventsSubject.next({ name: 'externalAction', data }); | ||
}, | ||
}, | ||
}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arent signals an array?
e.g.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a
signals
config that isn't directly tied to the spec. It's embedded within the existingkibana
field in the vega lite spec that's processed to handle OSD-specific items in the vega parser.From my understanding
signals
here is just a mapping of mark ID to asignals
array (that will be added to the compiledvega
spec later on)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is correct @ohltyler . In the vega parser we use the key "markId" to find the mark and then add then update the signal with that mark name.