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

[7.x] [Ingest Pipelines Editor] First round of UX improvements (#69381) #70076

Merged
merged 1 commit into from
Jun 26, 2020
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
3 changes: 2 additions & 1 deletion packages/kbn-monaco/src/xjson/grammar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,13 @@ export const createParser = () => {

try {
value();
white();
} catch (e) {
errored = true;
annos.push({ type: AnnoTypes.error, at: e.at - 1, text: e.message });
}
if (!errored && ch) {
error('Syntax error');
annos.push({ type: AnnoTypes.error, at: at, text: 'Syntax Error' });
}
return { annotations: annos };
}
Expand Down
5 changes: 4 additions & 1 deletion packages/kbn-monaco/src/xjson/language.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ export const registerGrammarChecker = (editor: monaco.editor.IEditor) => {

const updateAnnos = async () => {
const { annotations } = await wps.getAnnos();
const model = editor.getModel() as monaco.editor.ITextModel;
const model = editor.getModel() as monaco.editor.ITextModel | null;
if (!model) {
return;
}
monaco.editor.setModelMarkers(
model,
OWNER,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const PipelineForm: React.FunctionComponent<PipelineFormProps> = ({
});

const onEditorFlyoutOpen = useCallback(() => {
setIsTestingPipeline(false);
setIsRequestVisible(false);
}, [setIsRequestVisible]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@ jest.mock('@elastic/eui', () => {
}}
/>
),
// Mocking EuiCodeEditor, which uses React Ace under the hood
EuiCodeEditor: (props: any) => (
};
});

jest.mock('../../../../../../../../src/plugins/kibana_react/public', () => {
const original = jest.requireActual('../../../../../../../../src/plugins/kibana_react/public');
return {
...original,
// Mocking CodeEditor, which uses React Monaco under the hood
CodeEditor: (props: any) => (
<input
data-test-subj={props['data-test-subj'] || 'mockCodeEditor'}
data-currentvalue={props.value}
Expand Down Expand Up @@ -95,8 +102,9 @@ const createActions = (testBed: TestBed<TestSubject>) => {
act(() => {
find(`${processorSelector}.moveItemButton`).simulate('click');
});
component.update();
act(() => {
find(dropZoneSelector).last().simulate('click');
find(dropZoneSelector).simulate('click');
});
component.update();
},
Expand All @@ -122,13 +130,6 @@ const createActions = (testBed: TestBed<TestSubject>) => {
});
},

duplicateProcessor(processorSelector: string) {
find(`${processorSelector}.moreMenu.button`).simulate('click');
act(() => {
find(`${processorSelector}.moreMenu.duplicateButton`).simulate('click');
});
},

startAndCancelMove(processorSelector: string) {
act(() => {
find(`${processorSelector}.moveItemButton`).simulate('click');
Expand All @@ -139,6 +140,13 @@ const createActions = (testBed: TestBed<TestSubject>) => {
});
},

duplicateProcessor(processorSelector: string) {
find(`${processorSelector}.moreMenu.button`).simulate('click');
act(() => {
find(`${processorSelector}.moreMenu.duplicateButton`).simulate('click');
});
},

toggleOnFailure() {
find('pipelineEditorOnFailureToggle').simulate('click');
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
$dropZoneZIndex: 1; /* Prevent the next item down from obscuring the button */
$cancelButtonZIndex: 2;
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const OnFailureProcessorsTitle: FunctionComponent = () => {
<EuiText size="s" color="subdued">
<FormattedMessage
id="xpack.ingestPipelines.pipelineEditor.onFailureTreeDescription"
defaultMessage="The processors used to pre-process documents before indexing. {learnMoreLink}"
defaultMessage="The processors used to handle exceptions in this pipeline. {learnMoreLink}"
values={{
learnMoreLink: (
<EuiLink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import classNames from 'classnames';
import React, { FunctionComponent, useState } from 'react';

import { EuiContextMenuItem, EuiContextMenuPanel, EuiPopover, EuiButtonIcon } from '@elastic/eui';
Expand All @@ -12,6 +13,7 @@ import { editorItemMessages } from './messages';

interface Props {
disabled: boolean;
hidden: boolean;
showAddOnFailure: boolean;
onDuplicate: () => void;
onDelete: () => void;
Expand All @@ -20,9 +22,13 @@ interface Props {
}

export const ContextMenu: FunctionComponent<Props> = (props) => {
const { showAddOnFailure, onDuplicate, onAddOnFailure, onDelete, disabled } = props;
const { showAddOnFailure, onDuplicate, onAddOnFailure, onDelete, disabled, hidden } = props;
const [isOpen, setIsOpen] = useState<boolean>(false);

const containerClasses = classNames({
'pipelineProcessorsEditor__item--displayNone': hidden,
});

const contextMenuItems = [
<EuiContextMenuItem
data-test-subj="duplicateButton"
Expand Down Expand Up @@ -63,23 +69,25 @@ export const ContextMenu: FunctionComponent<Props> = (props) => {
].filter(Boolean) as JSX.Element[];

return (
<EuiPopover
data-test-subj={props['data-test-subj']}
anchorPosition="leftCenter"
panelPaddingSize="none"
isOpen={isOpen}
closePopover={() => setIsOpen(false)}
button={
<EuiButtonIcon
data-test-subj="button"
disabled={disabled}
onClick={() => setIsOpen((v) => !v)}
iconType="boxesHorizontal"
aria-label={editorItemMessages.moreButtonAriaLabel}
/>
}
>
<EuiContextMenuPanel items={contextMenuItems} />
</EuiPopover>
<div className={containerClasses}>
<EuiPopover
data-test-subj={props['data-test-subj']}
anchorPosition="leftCenter"
panelPaddingSize="none"
isOpen={isOpen}
closePopover={() => setIsOpen(false)}
button={
<EuiButtonIcon
data-test-subj="button"
disabled={disabled}
onClick={() => setIsOpen((v) => !v)}
iconType="boxesHorizontal"
aria-label={editorItemMessages.moreButtonAriaLabel}
/>
}
>
<EuiContextMenuPanel items={contextMenuItems} />
</EuiPopover>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import classNames from 'classnames';
import React, { FunctionComponent, useState, useEffect, useCallback } from 'react';
import { EuiFieldText, EuiText, keyCodes } from '@elastic/eui';

export interface Props {
placeholder: string;
ariaLabel: string;
onChange: (value: string) => void;
disabled: boolean;
text?: string;
}

export const InlineTextInput: FunctionComponent<Props> = ({
disabled,
placeholder,
text,
ariaLabel,
Expand All @@ -23,26 +25,17 @@ export const InlineTextInput: FunctionComponent<Props> = ({
const [isShowingTextInput, setIsShowingTextInput] = useState<boolean>(false);
const [textValue, setTextValue] = useState<string>(text ?? '');

const content = isShowingTextInput ? (
<EuiFieldText
controlOnly
fullWidth
compressed
value={textValue}
aria-label={ariaLabel}
className="pipelineProcessorsEditor__item__textInput"
inputRef={(el) => el?.focus()}
onChange={(event) => setTextValue(event.target.value)}
/>
) : (
<EuiText size="s" color="subdued">
{text || <em>{placeholder}</em>}
</EuiText>
);
const containerClasses = classNames('pipelineProcessorsEditor__item__textContainer', {
'pipelineProcessorsEditor__item__textContainer--notEditing': !isShowingTextInput && !disabled,
});

const submitChange = useCallback(() => {
setIsShowingTextInput(false);
onChange(textValue);
// Give any on blur handlers the chance to complete if the user is
// tabbing over this component.
setTimeout(() => {
setIsShowingTextInput(false);
onChange(textValue);
});
}, [setIsShowingTextInput, onChange, textValue]);

useEffect(() => {
Expand All @@ -62,14 +55,27 @@ export const InlineTextInput: FunctionComponent<Props> = ({
};
}, [isShowingTextInput, submitChange, setIsShowingTextInput]);

return (
<div
className="pipelineProcessorsEditor__item__textContainer"
tabIndex={0}
onFocus={() => setIsShowingTextInput(true)}
onBlur={submitChange}
>
{content}
return isShowingTextInput && !disabled ? (
<div className={`pipelineProcessorsEditor__item__textContainer ${containerClasses}`}>
<EuiFieldText
controlOnly
onBlur={submitChange}
fullWidth
compressed
value={textValue}
aria-label={ariaLabel}
className="pipelineProcessorsEditor__item__textInput"
inputRef={(el) => el?.focus()}
onChange={(event) => setTextValue(event.target.value)}
/>
</div>
) : (
<div className={containerClasses} tabIndex={0} onFocus={() => setIsShowingTextInput(true)}>
<EuiText size="s" color="subdued">
<div className="pipelineProcessorsEditor__item__description">
{text || <em>{placeholder}</em>}
</div>
</EuiText>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,9 @@ export const editorItemMessages = {
moveButtonLabel: i18n.translate('xpack.ingestPipelines.pipelineEditor.item.moveButtonLabel', {
defaultMessage: 'Move this processor',
}),
editorButtonLabel: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.item.editButtonAriaLabel',
{
defaultMessage: 'Edit this processor',
}
),
editButtonLabel: i18n.translate('xpack.ingestPipelines.pipelineEditor.item.editButtonAriaLabel', {
defaultMessage: 'Edit this processor',
}),
duplicateButtonLabel: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.item.moreMenu.duplicateButtonLabel',
{
Expand All @@ -31,7 +28,7 @@ export const editorItemMessages = {
cancelMoveButtonLabel: i18n.translate(
'xpack.ingestPipelines.pipelineEditor.item.cancelMoveButtonAriaLabel',
{
defaultMessage: 'Cancel moving this processor',
defaultMessage: 'Cancel move',
}
),
deleteButtonLabel: i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,57 @@
@import '../shared';

.pipelineProcessorsEditor__item {
transition: border-color 1s;
min-height: 50px;
&--selected {
border: 1px solid $euiColorPrimary;
}

&--displayNone {
display: none;
}

&--dimmed {
box-shadow: none;
}

// Remove the box-shadow on all nested items
.pipelineProcessorsEditor__item {
box-shadow: none !important;
}

&__processorTypeLabel {
line-height: $euiButtonHeightSmall;
}

&__textContainer {
padding: 4px;
border-radius: 2px;

transition: border-color .3s;
border: 2px solid #FFF;
transition: border-color 0.3s;
border: 2px solid transparent;

&:hover {
border: 2px solid $euiColorLightShade;
&--notEditing {
&:hover {
border: 2px solid $euiColorLightShade;
}
}
}

&__description {
overflow-x: hidden;
white-space: nowrap;
text-overflow: ellipsis;
max-width: 600px;
}

&__textInput {
height: 21px;
min-width: 100px;
min-width: 150px;
}

&__cancelMoveButton {
// Ensure that the cancel button is above the drop zones
z-index: $cancelButtonZIndex;
}
}
Loading