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

[Ingest Pipeline] Processor Editor Item Styling tweak #70786

Merged
merged 9 commits into from
Jul 7, 2020
Merged

[Ingest Pipeline] Processor Editor Item Styling tweak #70786

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
64f9b1f
Small styling tweaks to processor items
jloleysens Jul 6, 2020
f30529b
Commit code that moves the cancel move button 🤦🏼‍♂️
jloleysens Jul 6, 2020
41aadce
Do not completely hide the move button, prevent ui from jumping
jloleysens Jul 6, 2020
fc1da11
Update styling and UX of move button; EuiToggleButton
jloleysens Jul 6, 2020
95c270a
use cross icon for cancelling move
jloleysens Jul 6, 2020
37f7883
Merge branch 'master' into ingest-pipeline/minor-styling-tweak
elasticmachine Jul 7, 2020
e1b8fca
replace hard values with EUI values in SCSS
jloleysens Jul 7, 2020
366e76e
Address rerendering triggered by context
jloleysens Jul 7, 2020
4ade0ad
Similarly move use of context to settings form container
jloleysens Jul 7, 2020
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
7 changes: 4 additions & 3 deletions ...ion/components/pipeline_processors_editor/__jest__/pipeline_processors_editor.helpers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const createActions = (testBed: TestBed<TestSubject>) => {

moveProcessor(processorSelector: string, dropZoneSelector: string) {
act(() => {
find(`${processorSelector}.moveItemButton`).simulate('click');
find(`${processorSelector}.moveItemButton`).simulate('change');
});
component.update();
act(() => {
Expand Down Expand Up @@ -144,12 +144,13 @@ const createActions = (testBed: TestBed<TestSubject>) => {

startAndCancelMove(processorSelector: string) {
act(() => {
find(`${processorSelector}.moveItemButton`).simulate('click');
find(`${processorSelector}.moveItemButton`).simulate('change');
});
component.update();
act(() => {
find(`${processorSelector}.cancelMoveItemButton`).simulate('click');
find(`${processorSelector}.cancelMoveItemButton`).simulate('change');
});
component.update();
},

duplicateProcessor(processorSelector: string) {
Expand Down
2 changes: 1 addition & 1 deletion ...cation/components/pipeline_processors_editor/__jest__/pipeline_processors_editor.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ describe('Pipeline Editor', () => {
const processorSelector = 'processors>0';
actions.startAndCancelMove(processorSelector);
// Assert that we have exited move mode for this processor
expect(exists(`moveItemButton-${processorSelector}`));
expect(exists(`${processorSelector}.moveItemButton`)).toBe(true);
const [onUpdateResult] = onUpdate.mock.calls[onUpdate.mock.calls.length - 1];
const { processors } = onUpdateResult.getData();
// Assert that nothing has changed
Expand Down
26 changes: 15 additions & 11 deletions ...rs_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
.pipelineProcessorsEditor__item {
transition: border-color 1s;
min-height: 50px;

&--selected {
border: 1px solid $euiColorPrimary;
}
Expand All @@ -25,15 +26,13 @@
}

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

transition: border-color 0.3s;
border: 2px solid transparent;
cursor: text;
border-bottom: 1px dashed transparent;

&--notEditing {
border-bottom: 1px dashed $euiColorLightShade;
&:hover {
border: 2px solid $euiColorLightShade;
border-color: $euiColorMediumShade;
}
}
}
Expand All @@ -46,12 +45,17 @@
}

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

&__cancelMoveButton {
// Ensure that the cancel button is above the drop zones
z-index: $cancelButtonZIndex;
&__moveButton {
&:hover {
transform: none !important;
}
&--cancel {
// Ensure that the cancel button is above the drop zones
z-index: $cancelButtonZIndex;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use on of our EUI zIndex sass variables here?

}
}
}
60 changes: 37 additions & 23 deletions ...ors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import classNames from 'classnames';
import React, { FunctionComponent, memo } from 'react';
import {
EuiButtonIcon,
EuiButton,
EuiButtonToggle,
EuiFlexGroup,
EuiFlexItem,
EuiPanel,
Expand Down Expand Up @@ -78,9 +78,41 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
'pipelineProcessorsEditor__item--displayNone': isInMoveMode && !processor.options.description,
});

const cancelMoveButtonClasses = classNames('pipelineProcessorsEditor__item__cancelMoveButton', {
'pipelineProcessorsEditor__item--displayNone': !isMovingThisProcessor,
});
const renderMoveButton = () => {
const label = !isMovingThisProcessor
? i18nTexts.moveButtonLabel
: i18nTexts.cancelMoveButtonLabel;
const dataTestSubj = !isMovingThisProcessor ? 'moveItemButton' : 'cancelMoveItemButton';
const moveButtonClasses = classNames('pipelineProcessorsEditor__item__moveButton', {
'pipelineProcessorsEditor__item__moveButton--cancel': isMovingThisProcessor,
});
const icon = isMovingThisProcessor ? 'cross' : 'sortable';
const moveButton = (
<EuiButtonToggle
isEmpty={!isMovingThisProcessor}
fill={isMovingThisProcessor}
isIconOnly
iconType={icon}
data-test-subj={dataTestSubj}
size="s"
disabled={isDisabled && !isMovingThisProcessor}
label={label}
aria-label={label}
onChange={() => (!isMovingThisProcessor ? onMove() : onCancelMove())}
/>
);
// Remove the tooltip from the DOM to prevent it from lingering if the mouse leave event
// did not fire.
return (
<div className={moveButtonClasses}>
{!isInMoveMode ? (
<EuiToolTip content={i18nTexts.moveButtonLabel}>{moveButton}</EuiToolTip>
) : (
moveButton
)}
</div>
);
};

return (
<EuiPanel className={panelClasses} paddingSize="s">
Expand All @@ -93,6 +125,7 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
>
<EuiFlexItem>
<EuiFlexGroup gutterSize="m" alignItems="center" responsive={false}>
<EuiFlexItem grow={false}>{renderMoveButton()}</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText
className="pipelineProcessorsEditor__item__processorTypeLabel"
Expand Down Expand Up @@ -149,25 +182,6 @@ export const PipelineProcessorsEditorItem: FunctionComponent<Props> = memo(
</EuiToolTip>
)}
</EuiFlexItem>
<EuiFlexItem className={actionElementClasses} grow={false}>
{!isInMoveMode && (
<EuiToolTip content={i18nTexts.moveButtonLabel}>
<EuiButtonIcon
data-test-subj="moveItemButton"
size="s"
disabled={isDisabled}
aria-label={i18nTexts.moveButtonLabel}
onClick={onMove}
iconType="sortable"
/>
</EuiToolTip>
)}
</EuiFlexItem>
<EuiFlexItem grow={false} className={cancelMoveButtonClasses}>
<EuiButton data-test-subj="cancelMoveItemButton" size="s" onClick={onCancelMove}>
{i18nTexts.cancelMoveButtonLabel}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={false}>
Expand Down