[Ingest Pipeline] Processor Editor Item Styling tweak #70786

jloleysens · 2020-07-06T10:50:16Z

Summary

Very minor tweaks to the style of the processor editor items. Also includes a fix for a performance regression linked to editing values in the form flyout. This was addressed by adding a container component to pipeline_processors_editor_item.container.tsx and moving the use of context to processor_settings_form.container.tsx.

How to test

Start Kibana
Go to the Ingest Node Pipelines plugin in the management section

Screenshots

Before

Idle state

Editing inline state

Movings state

After

Idle and inline editing state

Moving state (unchanged)

- Moved the move button to the before the processor name - Cancel button is still after description if there is one - Made inline text description a bit taller and changed border style

elasticmachine · 2020-07-06T10:50:18Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

sebelga · 2020-07-06T12:35:23Z

Thanks for adding those change @jloleysens . I can't test it locally right now, but looking at the screenshot it seems that there will be a jump from "idle" to "moving" state as you remove the move icon on the left. Why not just dim the icon while in moving state to avoid this jump?

I personally don't like much the bold text for each of the processor types, this is why I suggested the badge as with the mappings editor. But I agree that this would need a bit more cycles and time to polish so we can leave it for later 👍

jloleysens · 2020-07-06T12:39:13Z

but looking at the screenshot it seems that there will be a jump from "idle" to "moving" state as you remove the move icon on the left. Why not just dim the icon while in moving state to avoid this jump?

Yeah I hear what you are saying, I tend to feel similarly. What are your thoughts on this @mdefazio ?

mdefazio · 2020-07-06T13:04:18Z

I agree that we should keep the move icon present (or in the same place). I was working through something similar to Seb's proposal where the move icon changes to an 'active' state. While having this as 'active' is maybe a bit specific to this prototype sketch, I think it would be nice to have the move icon act as the toggle to stop moving as well.

I think it is important as well that we don't have any jumps in either the processor names or the UI controls.

Screenshot is showing...
Default
Hover
Move processor selected

jloleysens · 2020-07-06T13:22:42Z

@sebelga @mdefazio Thanks for the feedback both! Here is the moving states with updates to prevent the UI from jumping:

[Edit was sharing an old image]

I went with the X icon as it feels more like a clear shout at the user what this state means. Thanks for recommendation regarding EuiToggleButton. Definitely the component I was looking for.

- Bring the styling of the button more in line with this comment elastic#70786 (comment)

jloleysens · 2020-07-07T09:02:10Z

@elasticmachine merge upstream

mdefazio

General design looks great!

Just some hard px values that I think we can swap out for eui variables that I've commented on.

mdefazio · 2020-07-07T11:46:51Z

...ssors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.scss

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


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

...ssors_editor/components/pipeline_processors_editor_item/pipeline_processors_editor_item.scss

- also prevent re-renders basded on contstructing objects on each render

We are only interested in the es docs path string in the settings form component so no need to render for other updates.

mdefazio

LGTM!

kibanamachine · 2020-07-07T16:18:20Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4ade0ad

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
ingestPipelines	258	+1	257

History

💚 Build #59299 succeeded 37f7883
💚 Build #58970 succeeded 95c270a
💔 Build #58945 failed 41aadce
💚 Build #58920 succeeded f30529b
💚 Build #58884 succeeded f30529b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Small styling tweaks to processor items - Moved the move button to the before the processor name - Cancel button is still after description if there is one - Made inline text description a bit taller and changed border style * Commit code that moves the cancel move button 🤦🏼‍♂️ * Do not completely hide the move button, prevent ui from jumping * Update styling and UX of move button; EuiToggleButton - Bring the styling of the button more in line with this comment elastic#70786 (comment) * use cross icon for cancelling move * replace hard values with EUI values in SCSS * Address rerendering triggered by context - also prevent re-renders basded on contstructing objects on each render * Similarly move use of context to settings form container We are only interested in the es docs path string in the settings form component so no need to render for other updates. Co-authored-by: Elastic Machine <[email protected]>

* Small styling tweaks to processor items - Moved the move button to the before the processor name - Cancel button is still after description if there is one - Made inline text description a bit taller and changed border style * Commit code that moves the cancel move button 🤦🏼‍♂️ * Do not completely hide the move button, prevent ui from jumping * Update styling and UX of move button; EuiToggleButton - Bring the styling of the button more in line with this comment #70786 (comment) * use cross icon for cancelling move * replace hard values with EUI values in SCSS * Address rerendering triggered by context - also prevent re-renders basded on contstructing objects on each render * Similarly move use of context to settings form container We are only interested in the es docs path string in the settings form component so no need to render for other updates. Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

jloleysens added 2 commits July 6, 2020 12:40

Small styling tweaks to processor items

64f9b1f

- Moved the move button to the before the processor name - Cancel button is still after description if there is one - Made inline text description a bit taller and changed border style

Commit code that moves the cancel move button 🤦🏼‍♂️

f30529b

jloleysens added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.9.0 Feature:Ingest Node Pipelines Ingest node pipelines management labels Jul 6, 2020

jloleysens requested review from sebelga and mdefazio July 6, 2020 10:50

jloleysens requested a review from a team as a code owner July 6, 2020 10:50

Do not completely hide the move button, prevent ui from jumping

41aadce

jloleysens added 2 commits July 6, 2020 16:16

Update styling and UX of move button; EuiToggleButton

fc1da11

- Bring the styling of the button more in line with this comment elastic#70786 (comment)

use cross icon for cancelling move

95c270a

Merge branch 'master' into ingest-pipeline/minor-styling-tweak

37f7883

mdefazio requested changes Jul 7, 2020

View reviewed changes

jloleysens added 3 commits July 7, 2020 14:45

replace hard values with EUI values in SCSS

e1b8fca

Address rerendering triggered by context

366e76e

- also prevent re-renders basded on contstructing objects on each render

Similarly move use of context to settings form container

4ade0ad

We are only interested in the es docs path string in the settings form component so no need to render for other updates.

mdefazio approved these changes Jul 7, 2020

View reviewed changes

jloleysens merged commit 50a2991 into elastic:master Jul 7, 2020

jloleysens deleted the ingest-pipeline/minor-styling-tweak branch July 7, 2020 17:23

jloleysens mentioned this pull request Jul 7, 2020

[7.x] [Ingest Pipeline] Processor Editor Item Styling tweak (#70786) #70991

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Ingest Pipeline] Processor Editor Item Styling tweak #70786

[Ingest Pipeline] Processor Editor Item Styling tweak #70786

jloleysens commented Jul 6, 2020 •

edited

Loading

elasticmachine commented Jul 6, 2020

sebelga commented Jul 6, 2020

jloleysens commented Jul 6, 2020

mdefazio commented Jul 6, 2020

jloleysens commented Jul 6, 2020 •

edited

Loading

jloleysens commented Jul 7, 2020

mdefazio left a comment

mdefazio Jul 7, 2020

mdefazio left a comment

kibanamachine commented Jul 7, 2020

[Ingest Pipeline] Processor Editor Item Styling tweak #70786

[Ingest Pipeline] Processor Editor Item Styling tweak #70786

Conversation

jloleysens commented Jul 6, 2020 • edited Loading

Summary

How to test

Screenshots

Before

After

elasticmachine commented Jul 6, 2020

sebelga commented Jul 6, 2020

jloleysens commented Jul 6, 2020

mdefazio commented Jul 6, 2020

jloleysens commented Jul 6, 2020 • edited Loading

jloleysens commented Jul 7, 2020

mdefazio left a comment

Choose a reason for hiding this comment

mdefazio Jul 7, 2020

Choose a reason for hiding this comment

mdefazio left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 7, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

History

jloleysens commented Jul 6, 2020 •

edited

Loading

jloleysens commented Jul 6, 2020 •

edited

Loading