From fb47cca3ebe60bc14b55fbc8e17f24e6ccd5621b Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Tue, 29 Oct 2024 09:41:27 -0700 Subject: [PATCH] Misc bug fixes and minor UX improvements II (#432) (#439) * Change readonly fields into codeblock * Change which panel is collapsible; various wording changes * Move edit buttons of ingest/search in header; spacing cleanup * Update wording and layout of input/output transform form components * update tests * remove gutter entirely on query buttons --------- Signed-off-by: Tyler Ohlsen --- common/constants.ts | 1 - .../general_components/processors_title.tsx | 9 +- .../workflow_detail/components/header.tsx | 7 - .../workflow_detail/resizable_workspace.tsx | 30 ++-- public/pages/workflow_detail/tools/tools.tsx | 7 +- .../workflow_detail/workflow_detail.test.tsx | 7 +- .../ingest_inputs/source_data.tsx | 69 +++++--- .../input_fields/map_array_field.tsx | 31 ++-- .../input_fields/map_field.tsx | 165 +++++++++++------- .../processor_inputs/ml_processor_inputs.tsx | 52 +++--- .../modals/input_transform_modal.tsx | 15 +- .../modals/output_transform_modal.tsx | 18 +- .../workflow_inputs/processors_list.tsx | 2 +- .../configure_search_request.tsx | 137 ++++++++------- 14 files changed, 325 insertions(+), 225 deletions(-) diff --git a/common/constants.ts b/common/constants.ts index 4d03e595..9b3880e1 100644 --- a/common/constants.ts +++ b/common/constants.ts @@ -456,7 +456,6 @@ export enum PROCESSOR_CONTEXT { export const START_FROM_SCRATCH_WORKFLOW_NAME = 'Start From Scratch'; export const DEFAULT_NEW_WORKFLOW_NAME = 'new_workflow'; export const DEFAULT_NEW_WORKFLOW_DESCRIPTION = 'My new workflow'; -export const DEFAULT_NEW_WORKFLOW_STATE = WORKFLOW_STATE.NOT_STARTED; export const DEFAULT_NEW_WORKFLOW_STATE_TYPE = ('NOT_STARTED' as any) as typeof WORKFLOW_STATE; export const DATE_FORMAT_PATTERN = 'MM/DD/YY hh:mm A'; export const EMPTY_FIELD_STRING = '--'; diff --git a/public/general_components/processors_title.tsx b/public/general_components/processors_title.tsx index feebcf82..fd718e5a 100644 --- a/public/general_components/processors_title.tsx +++ b/public/general_components/processors_title.tsx @@ -16,9 +16,12 @@ interface ProcessorsTitleProps { */ export function ProcessorsTitle(props: ProcessorsTitleProps) { return ( - + -
+ <>

{`${props.title} (${props.processorCount}) -`}

@@ -26,7 +29,7 @@ export function ProcessorsTitle(props: ProcessorsTitleProps) {

optional

-
+ ); diff --git a/public/pages/workflow_detail/components/header.tsx b/public/pages/workflow_detail/components/header.tsx index 0df72f1e..0d5dc423 100644 --- a/public/pages/workflow_detail/components/header.tsx +++ b/public/pages/workflow_detail/components/header.tsx @@ -14,10 +14,8 @@ import { EuiSmallButton, } from '@elastic/eui'; import { - DEFAULT_NEW_WORKFLOW_STATE, PLUGIN_ID, MAX_WORKFLOW_NAME_TO_DISPLAY, - WORKFLOW_STATE, Workflow, getCharacterLimitedString, toFormattedDate, @@ -58,7 +56,6 @@ export function WorkflowDetailHeader(props: WorkflowDetailHeaderProps) { const history = useHistory(); // workflow state const [workflowName, setWorkflowName] = useState(''); - const [workflowState, setWorkflowState] = useState(''); const [workflowLastUpdated, setWorkflowLastUpdated] = useState(''); // export modal state @@ -80,7 +77,6 @@ export function WorkflowDetailHeader(props: WorkflowDetailHeaderProps) { MAX_WORKFLOW_NAME_TO_DISPLAY ) ); - setWorkflowState(props.workflow.state || DEFAULT_NEW_WORKFLOW_STATE); try { const formattedDate = toFormattedDate( // @ts-ignore @@ -206,9 +202,6 @@ export function WorkflowDetailHeader(props: WorkflowDetailHeaderProps) { pageTitle={ {workflowName} - - {workflowState} - } rightSideItems={[ diff --git a/public/pages/workflow_detail/resizable_workspace.tsx b/public/pages/workflow_detail/resizable_workspace.tsx index d6a96bd1..732ab399 100644 --- a/public/pages/workflow_detail/resizable_workspace.tsx +++ b/public/pages/workflow_detail/resizable_workspace.tsx @@ -41,6 +41,7 @@ interface ResizableWorkspaceProps { } const WORKFLOW_INPUTS_PANEL_ID = 'workflow_inputs_panel_id'; +const PREVIEW_PANEL_ID = 'preview_panel_id'; const TOOLS_PANEL_ID = 'tools_panel_id'; /** @@ -69,18 +70,16 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) { undefined ); - // Workflow inputs side panel state - const [isWorkflowInputsPanelOpen, setIsWorkflowInputsPanelOpen] = useState< - boolean - >(true); + // Preview side panel state. This panel encapsulates the tools panel as a child resizable panel. + const [isPreviewPanelOpen, setIsPreviewPanelOpen] = useState(true); const collapseFnHorizontal = useRef( (id: string, options: { direction: 'left' | 'right' }) => {} ); - const onToggleWorkflowInputsChange = () => { - collapseFnHorizontal.current(WORKFLOW_INPUTS_PANEL_ID, { - direction: 'left', + const onTogglePreviewChange = () => { + collapseFnHorizontal.current(PREVIEW_PANEL_ID, { + direction: 'right', }); - setIsWorkflowInputsPanelOpen(!isWorkflowInputsPanelOpen); + setIsPreviewPanelOpen(!isPreviewPanelOpen); }; // ingest state @@ -143,7 +142,7 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) { direction="horizontal" className="stretch-absolute" style={{ - marginLeft: isWorkflowInputsPanelOpen ? '-8px' : '0px', + marginLeft: '-8px', marginTop: '-8px', }} > @@ -159,13 +158,10 @@ export function ResizableWorkspace(props: ResizableWorkspaceProps) { <> - onToggleWorkflowInputsChange() - } > onTogglePreviewChange()} > - + -

Tools

+

{PANEL_TITLE}

 diff --git a/public/pages/workflow_detail/workflow_detail.test.tsx b/public/pages/workflow_detail/workflow_detail.test.tsx index 8e62cd6e..b5c4f237 100644 --- a/public/pages/workflow_detail/workflow_detail.test.tsx +++ b/public/pages/workflow_detail/workflow_detail.test.tsx @@ -83,9 +83,8 @@ describe('WorkflowDetail Page with create ingestion option', () => { ); expect(getAllByText('Skip ingestion pipeline').length).toBeGreaterThan(0); expect(getAllByText('Define ingest pipeline').length).toBeGreaterThan(0); - expect(getAllByText('Tools').length).toBeGreaterThan(0); + expect(getAllByText('Inspector').length).toBeGreaterThan(0); expect(getAllByText('Preview').length).toBeGreaterThan(0); - expect(getAllByText('Not started').length).toBeGreaterThan(0); expect( getAllByText((content) => content.startsWith('Last updated:')).length ).toBeGreaterThan(0); @@ -240,14 +239,14 @@ describe('WorkflowDetail Page with skip ingestion option (Hybrid Search Workflow ); userEvent.click(addRequestProcessorButton); await waitFor(() => { - expect(getAllByText('Processors').length).toBeGreaterThan(0); + expect(getAllByText('PROCESSORS').length).toBeGreaterThan(0); }); // Add response processor const addResponseProcessorButton = getAllByTestId('addProcessorButton')[1]; userEvent.click(addResponseProcessorButton); await waitFor(() => { - expect(getAllByText('Processors').length).toBeGreaterThan(0); + expect(getAllByText('PROCESSORS').length).toBeGreaterThan(0); }); // Save, Build and Run query, Back buttons diff --git a/public/pages/workflow_detail/workflow_inputs/ingest_inputs/source_data.tsx b/public/pages/workflow_detail/workflow_inputs/ingest_inputs/source_data.tsx index e0418986..c905ddec 100644 --- a/public/pages/workflow_detail/workflow_inputs/ingest_inputs/source_data.tsx +++ b/public/pages/workflow_detail/workflow_inputs/ingest_inputs/source_data.tsx @@ -22,6 +22,8 @@ import { EuiSmallFilterButton, EuiSuperSelectOption, EuiCompressedSuperSelect, + EuiCodeBlock, + EuiSmallButtonEmpty, } from '@elastic/eui'; import { JsonField } from '../input_fields'; import { @@ -293,9 +295,25 @@ export function SourceData(props: SourceDataProps) { )} - -

Source data

- + + + +

Import data

+ + + {docsPopulated && ( + + setIsEditModalOpen(true)} + data-testid="editSourceDataButton" + iconType="pencil" + iconSide="left" + > + Edit + + + )} + {props.lastIngested !== undefined && ( @@ -305,27 +323,36 @@ export function SourceData(props: SourceDataProps) { )} - - setIsEditModalOpen(true)} - data-testid="editSourceDataButton" - > - {docsPopulated ? `Edit` : `Select data`} - - - {docsPopulated && ( + {!docsPopulated && ( - + setIsEditModalOpen(true)} + data-testid="selectDataToImportButton" + > + {`Select data`} + )} + + {docsPopulated && ( + <> + + + + {getIn(values, 'ingest.docs')} + + + + )} ); diff --git a/public/pages/workflow_detail/workflow_inputs/input_fields/map_array_field.tsx b/public/pages/workflow_detail/workflow_inputs/input_fields/map_array_field.tsx index 6971e65e..5daf11f2 100644 --- a/public/pages/workflow_detail/workflow_inputs/input_fields/map_array_field.tsx +++ b/public/pages/workflow_detail/workflow_inputs/input_fields/map_array_field.tsx @@ -10,12 +10,10 @@ import { EuiFlexGroup, EuiFlexItem, EuiCompressedFormRow, - EuiLink, EuiPanel, EuiSmallButton, EuiSmallButtonEmpty, EuiSpacer, - EuiText, } from '@elastic/eui'; import { Field, FieldProps, getIn, useFormikContext } from 'formik'; import { @@ -30,15 +28,17 @@ import { MapField } from './map_field'; interface MapArrayFieldProps { field: IConfigField; fieldPath: string; // the full path in string-form to the field (e.g., 'ingest.enrich.processors.text_embedding_processor.inputField') - label?: string; - helpLink?: string; helpText?: string; + keyTitle?: string; keyPlaceholder?: string; + valueTitle?: string; valuePlaceholder?: string; onMapAdd?: (curArray: MapArrayFormValue) => void; onMapDelete?: (idxToDelete: number) => void; keyOptions?: { label: string }[]; valueOptions?: { label: string }[]; + addMapEntryButtonText?: string; + addMapButtonText?: string; } /** @@ -90,17 +90,6 @@ export function MapArrayField(props: MapArrayFieldProps) { - - Learn more - - - ) : undefined - } - helpText={props.helpText || undefined} isInvalid={ getIn(errors, field.name) !== undefined && getIn(errors, field.name).length > 0 && @@ -135,10 +124,14 @@ export function MapArrayField(props: MapArrayFieldProps) { @@ -151,10 +144,14 @@ export function MapArrayField(props: MapArrayFieldProps) { @@ -182,7 +179,9 @@ export function MapArrayField(props: MapArrayFieldProps) { onClick={() => { addMap(field.value); }} - >{`(Advanced) Add another prediction`} + > + {props.addMapButtonText || `(Advanced) Add another map`} + )} diff --git a/public/pages/workflow_detail/workflow_inputs/input_fields/map_field.tsx b/public/pages/workflow_detail/workflow_inputs/input_fields/map_field.tsx index 8d026d83..be8fd2a4 100644 --- a/public/pages/workflow_detail/workflow_inputs/input_fields/map_field.tsx +++ b/public/pages/workflow_detail/workflow_inputs/input_fields/map_field.tsx @@ -13,6 +13,7 @@ import { EuiLink, EuiText, EuiSmallButton, + EuiIconTip, } from '@elastic/eui'; import { Field, FieldProps, getIn, useFormikContext } from 'formik'; import { isEmpty } from 'lodash'; @@ -29,12 +30,20 @@ interface MapFieldProps { label?: string; helpLink?: string; helpText?: string; + keyTitle?: string; keyPlaceholder?: string; + valueTitle?: string; valuePlaceholder?: string; keyOptions?: { label: string }[]; valueOptions?: { label: string }[]; + addEntryButtonText?: string; } +// The keys will be more static in general. Give more space for values where users +// will typically be writing out more complex transforms/configuration (in the case of ML inference processors). +const KEY_FLEX_RATIO = 4; +const VALUE_FLEX_RATIO = 6; + /** * Input component for configuring field mappings. Input forms are defaulted to text fields. If * keyOptions or valueOptions are set, set the respective input form as a select field, with those options. @@ -80,7 +89,6 @@ export function MapField(props: MapFieldProps) { ) : undefined } - helpText={props.helpText || undefined} error={ getIn(errors, field.name) !== undefined && getIn(errors, field.name).length > 0 @@ -95,69 +103,108 @@ export function MapField(props: MapFieldProps) { } > + + + + + {props.keyTitle || 'Key'} + + + + + + + {props.valueTitle || 'Value'} + + + {props.helpText && ( + + + + )} + + + + + {field.value?.map((mapping: MapEntry, idx: number) => { return ( - - + + - - <> - {!isEmpty(props.keyOptions) ? ( - - ) : ( - - )} - - - - - - - <> - {!isEmpty(props.valueOptions) ? ( - - ) : ( - - )} - - + <> + + <> + {!isEmpty(props.keyOptions) ? ( + + ) : ( + + )} + + + + + + - - { - deleteMapEntry(field.value, idx); - }} - /> + + + <> + + <> + {!isEmpty(props.valueOptions) ? ( + + ) : ( + + )} + + + + { + deleteMapEntry(field.value, idx); + }} + /> + + + @@ -170,7 +217,7 @@ export function MapField(props: MapFieldProps) { addMapEntry(field.value); }} > - {field.value?.length > 0 ? 'Add more' : 'Add field mapping'} + {props.addEntryButtonText || 'Add more'} diff --git a/public/pages/workflow_detail/workflow_inputs/processor_inputs/ml_processor_inputs.tsx b/public/pages/workflow_detail/workflow_inputs/processor_inputs/ml_processor_inputs.tsx index 3803a2aa..39c7f649 100644 --- a/public/pages/workflow_detail/workflow_inputs/processor_inputs/ml_processor_inputs.tsx +++ b/public/pages/workflow_detail/workflow_inputs/processor_inputs/ml_processor_inputs.tsx @@ -336,16 +336,12 @@ export function MLProcessorInputs(props: MLProcessorInputsProps) { )} - + {`Configure input transformations (${ - props.context === PROCESSOR_CONTEXT.SEARCH_REQUEST - ? 'Required' - : 'Optional' - })`} + >{`Inputs`} { setIsInputTransformModalOpen(true); }} > - Preview + Preview inputs @@ -371,17 +367,21 @@ export function MLProcessorInputs(props: MLProcessorInputsProps) { - + {`Configure output transformations (${ - props.context === PROCESSOR_CONTEXT.SEARCH_REQUEST - ? 'Required' - : 'Optional' - })`} + >{`Outputs`} { setIsOutputTransformModalOpen(true); }} > - Preview + Preview outputs @@ -426,21 +424,27 @@ export function MLProcessorInputs(props: MLProcessorInputsProps) { {inputMapValue.length !== outputMapValue.length && diff --git a/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/input_transform_modal.tsx b/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/input_transform_modal.tsx index 610beeee..6a75e53c 100644 --- a/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/input_transform_modal.tsx +++ b/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/input_transform_modal.tsx @@ -34,7 +34,6 @@ import { IProcessorConfig, IngestPipelineConfig, JSONPATH_ROOT_SELECTOR, - ML_INFERENCE_DOCS_LINK, ML_INFERENCE_RESPONSE_DOCS_LINK, MapArrayFormValue, ModelInterface, @@ -441,17 +440,21 @@ export function InputTransformModal(props: InputTransformModalProps) { { @@ -467,6 +470,8 @@ export function InputTransformModal(props: InputTransformModalProps) { setTransformedInput('{}'); } }} + addMapEntryButtonText="Add input" + addMapButtonText="(Advanced) Add input group" /> diff --git a/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/output_transform_modal.tsx b/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/output_transform_modal.tsx index c877c131..ab7aaf5c 100644 --- a/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/output_transform_modal.tsx +++ b/public/pages/workflow_detail/workflow_inputs/processor_inputs/modals/output_transform_modal.tsx @@ -388,12 +388,20 @@ export function OutputTransformModal(props: OutputTransformModalProps) { diff --git a/public/pages/workflow_detail/workflow_inputs/processors_list.tsx b/public/pages/workflow_detail/workflow_inputs/processors_list.tsx index 4e70c105..89ea1b17 100644 --- a/public/pages/workflow_detail/workflow_inputs/processors_list.tsx +++ b/public/pages/workflow_detail/workflow_inputs/processors_list.tsx @@ -209,7 +209,7 @@ export function ProcessorsList(props: ProcessorsListProps) { panels={[ { id: PANEL_ID, - title: 'Processors', + title: 'PROCESSORS', items: props.context === PROCESSOR_CONTEXT.INGEST ? [ diff --git a/public/pages/workflow_detail/workflow_inputs/search_inputs/configure_search_request.tsx b/public/pages/workflow_detail/workflow_inputs/search_inputs/configure_search_request.tsx index aff505e1..b71e988e 100644 --- a/public/pages/workflow_detail/workflow_inputs/search_inputs/configure_search_request.tsx +++ b/public/pages/workflow_detail/workflow_inputs/search_inputs/configure_search_request.tsx @@ -5,9 +5,8 @@ import React, { useEffect, useState } from 'react'; import { useSelector } from 'react-redux'; -import { useFormikContext } from 'formik'; +import { getIn, useFormikContext } from 'formik'; import { - EuiSmallButton, EuiCompressedFieldText, EuiFlexGroup, EuiFlexItem, @@ -15,14 +14,14 @@ import { EuiCompressedSuperSelect, EuiSuperSelectOption, EuiText, - EuiSpacer, + EuiCodeBlock, + EuiSmallButtonEmpty, } from '@elastic/eui'; import { SearchHit, WorkflowFormValues, customStringify, } from '../../../../../common'; -import { JsonField } from '../input_fields'; import { AppState, searchIndex, useAppDispatch } from '../../../../store'; import { getDataSourceId } from '../../../../utils/utils'; import { EditQueryModal } from './edit_query_modal'; @@ -80,7 +79,7 @@ export function ConfigureSearchRequest(props: ConfigureSearchRequestProps) { queryFieldPath="search.request" /> )} - +

Configure query

@@ -114,64 +113,78 @@ export function ConfigureSearchRequest(props: ConfigureSearchRequestProps) { )} - - setIsEditModalOpen(true)} - data-testid="queryEditButton" - > - Edit - - - - - - - <> - { - // for this test query, we don't want to involve any configured search pipelines, if any exist - // see https://opensearch.org/docs/latest/search-plugins/search-pipelines/using-search-pipeline/#disabling-the-default-pipeline-for-a-request - dispatch( - searchIndex({ - apiBody: { - index: values.search.index.name, - body: values.search.request, - searchPipeline: '_none', - }, - dataSourceId, - }) - ) - .unwrap() - .then(async (resp) => { - props.setQueryResponse( - customStringify( - resp.hits.hits.map((hit: SearchHit) => hit._source) + + + + +

Query definition

+ + + + + + setIsEditModalOpen(true)} + data-testid="queryEditButton" + iconType="pencil" + iconSide="left" + > + Edit + + + + { + // for this test query, we don't want to involve any configured search pipelines, if any exist + // see https://opensearch.org/docs/latest/search-plugins/search-pipelines/using-search-pipeline/#disabling-the-default-pipeline-for-a-request + dispatch( + searchIndex({ + apiBody: { + index: values.search.index.name, + body: values.search.request, + searchPipeline: '_none', + }, + dataSourceId, + }) ) - ); - }) - .catch((error: any) => { - props.setQueryResponse(''); - console.error('Error running query: ', error); - }); - }} - data-testid="searchTestButton" - > - Test - - - - Run query without any search pipeline configuration. - - + .unwrap() + .then(async (resp) => { + props.setQueryResponse( + customStringify( + resp.hits.hits.map( + (hit: SearchHit) => hit._source + ) + ) + ); + }) + .catch((error: any) => { + props.setQueryResponse(''); + console.error('Error running query: ', error); + }); + }} + data-testid="searchTestButton" + iconType="play" + iconSide="left" + > + Test query + + + + + + + + + {getIn(values, 'search.request')} +