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.

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

T2viz ux improvements #330

Merged
merged 5 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- feat: support navigating to discover in alerting popover([#316](https://github.com/opensearch-project/dashboards-assistant/pull/316))
- fix: incorrect string escaping of vega schema([325](https://github.com/opensearch-project/dashboards-assistant/pull/325))
- feat: register the AI actions to query controls in discover([#327](https://github.com/opensearch-project/dashboards-assistant/pull/327))
- fix: t2viz ux improvements([#330](https://github.com/opensearch-project/dashboards-assistant/pull/330))

### 📈 Features/Enhancements

Expand Down
3 changes: 2 additions & 1 deletion common/types/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export const configSchema = schema.object({
enabled: schema.boolean({ defaultValue: false }),
}),
branding: schema.object({
label: schema.string({ defaultValue: '' }),
label: schema.maybe(schema.string()),
logo: schema.maybe(schema.string()),
}),
});

Expand Down
1 change: 1 addition & 0 deletions public/components/ui_action_context_menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const ActionContextMenu = (props: Props) => {
trigger: AI_ASSISTANT_QUERY_EDITOR_TRIGGER as any,
})),
closeMenu: () => setOpen(false),
title: '',
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this empty title is needed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the default title is Options but we don't need a title, so set it to ''

}),
[]
);
Expand Down
57 changes: 34 additions & 23 deletions public/components/visualization/source_selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useCallback, useMemo, useState, useEffect } from 'react';
import React, { useCallback, useMemo, useState, useEffect, useRef } from 'react';
import { i18n } from '@osd/i18n';

import { useOpenSearchDashboards } from '../../../../../src/plugins/opensearch_dashboards_react/public';
Expand Down Expand Up @@ -38,6 +38,8 @@ export const SourceSelector = ({
} = useOpenSearchDashboards<StartServices>();
const [currentDataSources, setCurrentDataSources] = useState<DataSource[]>([]);
const [dataSourceOptions, setDataSourceOptions] = useState<DataSourceGroup[]>([]);
const onChangeRef = useRef(onChange);
onChangeRef.current = onChange;

const selectedSources = useMemo(() => {
if (selectedSourceId) {
Expand All @@ -52,13 +54,20 @@ export const SourceSelector = ({
return [];
}, [selectedSourceId, dataSourceOptions]);

/**
* When initialized, select the first non-disabled option
*/
useEffect(() => {
if (
!selectedSourceId &&
dataSourceOptions.length > 0 &&
dataSourceOptions[0].options.length > 0
) {
onChange(dataSourceOptions[0].options[0]);
const options = dataSourceOptions[0].options;
const selectedOption = options.find((o) => !o.disabled);
if (selectedOption) {
onChangeRef.current(selectedOption);
}
}
}, [selectedSourceId, dataSourceOptions]);

Expand All @@ -81,7 +90,7 @@ export const SourceSelector = ({

const onSetDataSourceOptions = useCallback(
async (options: DataSourceGroup[]) => {
// Only support opensearch default data source
// Only support OpenSearch default data source
const indexPatternOptions = options.find(
(item) => item.groupType === DEFAULT_DATA_SOURCE_TYPE
);
Expand Down Expand Up @@ -117,28 +126,30 @@ export const SourceSelector = ({
* Check each data source to see if text to vega agents are configured or not
* If not configured, disable the corresponding index pattern from the selection list
*/
Object.keys(dataSourceIdToIndexPatternIds).forEach(async (key) => {
const res = await assistantService.client.agentConfigExists(
[
TEXT2VEGA_RULE_BASED_AGENT_CONFIG_ID,
TEXT2VEGA_WITH_INSTRUCTIONS_AGENT_CONFIG_ID,
TEXT2PPL_AGENT_CONFIG_ID,
],
{
dataSourceId: key !== 'DEFAULT' ? key : undefined,
}
);
if (!res.exists) {
dataSourceIdToIndexPatternIds[key].forEach((indexPatternId) => {
indexPatternOptions.options.forEach((option) => {
if (option.value === indexPatternId) {
option.disabled = true;
}
const updateIndexPatternPromises = Object.keys(dataSourceIdToIndexPatternIds).map(
async (key) => {
const res = await assistantService.client.agentConfigExists(
[
TEXT2VEGA_RULE_BASED_AGENT_CONFIG_ID,
TEXT2VEGA_WITH_INSTRUCTIONS_AGENT_CONFIG_ID,
TEXT2PPL_AGENT_CONFIG_ID,
],
{
dataSourceId: key !== 'DEFAULT' ? key : undefined,
}
);
if (!res.exists) {
dataSourceIdToIndexPatternIds[key].forEach((indexPatternId) => {
indexPatternOptions.options.forEach((option) => {
if (option.value === indexPatternId) {
option.disabled = true;
}
});
});
});
}
}
});

);
await Promise.allSettled(updateIndexPatternPromises);
setDataSourceOptions([indexPatternOptions]);
},
[currentDataSources]
Expand Down
3 changes: 3 additions & 0 deletions public/components/visualization/text2vega.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ export class Text2Vega {
)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
.toPromise<any>();
if (res.rawResponse.total === 0) {
throw new Error(`There is no result with the generated query: '${value.ppl}'.`);
}
Comment on lines +66 to +68
Copy link
Collaborator

Choose a reason for hiding this comment

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

would there a place to show this error on page?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, in text2viz.tsx line 110

return { ...value, sample: res.rawResponse };
}),
// call llm to generate vega
Expand Down
10 changes: 6 additions & 4 deletions public/components/visualization/text2viz.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export const Text2Viz = () => {
data,
uiSettings,
savedObjects,
config,
},
} = useOpenSearchDashboards<StartServices>();

Expand Down Expand Up @@ -106,7 +107,7 @@ export const Text2Viz = () => {
if (result.error) {
notifications.toasts.addError(result.error, {
title: i18n.translate('dashboardAssistant.feature.text2viz.error', {
defaultMessage: 'Error while executing text to vega',
defaultMessage: 'Error while executing text to visualization',
}),
});
} else {
Expand Down Expand Up @@ -198,7 +199,7 @@ export const Text2Viz = () => {
});

setSubmitting(false);
}, [selectedSource, input, status]);
}, [selectedSource, input, status, notifications.toasts]);

/**
* Display the save visualization dialog to persist the current generated visualization
Expand Down Expand Up @@ -342,16 +343,17 @@ export const Text2Viz = () => {
onChange={(e) => setInput(e.target.value)}
fullWidth
compressed
prepend={<EuiIcon type={chatIcon} />}
prepend={<EuiIcon type={config.branding.logo || chatIcon} />}
Copy link
Collaborator

Choose a reason for hiding this comment

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

will config.branding.logo supports svg file?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, url to svg is supported

placeholder="Generate visualization with a natural language question."
onKeyDown={(e) => e.key === 'Enter' && onSubmit()}
disabled={!selectedSource}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon
aria-label="submit"
onClick={onSubmit}
isDisabled={loading || input.trim().length === 0}
isDisabled={loading || input.trim().length === 0 || !selectedSource}
display="base"
size="s"
iconType="returnKey"
Expand Down
1 change: 1 addition & 0 deletions public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ export class AssistantPlugin
...pluginsStart,
...coreStart,
setHeaderActionMenu: params.setHeaderActionMenu,
config: this.config,
});

return () => {
Expand Down
2 changes: 2 additions & 0 deletions public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ExpressionsSetup, ExpressionsStart } from '../../../src/plugins/express
import { SavedObjectsStart } from '../../../src/plugins/saved_objects/public';

import { UsageCollectionSetup } from '../../../src/plugins/usage_collection/public';
import { ConfigSchema } from '../common/types/config';

export interface RenderProps {
props: MessageContentProps;
Expand Down Expand Up @@ -95,6 +96,7 @@ export interface AssistantStart {
export type StartServices = CoreStart &
Omit<AssistantPluginStartDependencies, 'savedObjects'> & {
setHeaderActionMenu: AppMountParameters['setHeaderActionMenu'];
config: ConfigSchema;
};

export interface UserAccount {
Expand Down
Loading