[Ingest pipelines] Address copy feedback #65175

alisonelizabeth · 2020-05-04T20:35:11Z

This PR updates copy in the Ingest Node Pipelines app.

Screenshots

elasticmachine · 2020-05-04T20:35:13Z

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

alisonelizabeth · 2020-05-05T01:36:06Z

...pelines/public/application/components/pipeline_form/pipeline_test_flyout/tabs/tab_output.tsx

          <FormattedMessage
            id="xpack.ingestPipelines.testPipelineFlyout.outputTab.descriptionText"
-            defaultMessage="The output of the executed pipeline. {runLink}"
+            defaultMessage="View the final output data, or see how each processor affects the ingest document as it passes through the pipeline. {runLink}"


@gchaps Let me know what you think of this (screenshot in initial PR comment). This differs from your original suggestion, as the View verbose output is actually disabled by default.

…nes/copy

cjcenizal

Looks great overall! I had a couple minor concerns which I think we should address before merging.

cjcenizal · 2020-05-05T16:13:25Z

x-pack/plugins/ingest_pipelines/public/application/app.tsx

+                          return (
+                            <>
+                              {acc}{' '}
+                              <FormattedMessage


I think it's worth checking with the i18n folks, but I don't think that we can depend on all languages following the English structure of using a word ("and" in the case of English) to join the last item onto a list. It might be safer to use the original pattern of simply joining on a comma.

That's a good point. I reverted the change to the original implementation. This also follows the pattern we're using in Snapshot + Restore, so it will be consistent.

cjcenizal · 2020-05-05T16:18:13Z

x-pack/plugins/ingest_pipelines/public/application/sections/pipelines_list/main.tsx

+              title={
+                <FormattedMessage
+                  id="xpack.ingestPipelines.list.loadErrorTitle"
+                  defaultMessage="Unable to load pipelines. Try {reloadLink} the page."


The way this text is phrased, it seems like clicking the reload link will reload the page, which I think of as a browser page refresh. But clicking this link will actually just resend the request to load the pipelines. Can we align text with action more closely by phrasing this as "Try reloading them"?

Good point! I went with Gail's suggestion: "Try again".

gchaps · 2020-05-05T16:46:58Z

I'm going to rescind one of my earlier comments. I think the text reads better as "Provide an array of documents for the pipeline to ingest."

Per @cjcenizal, how about:

Unable to load pipelines. Try again.

and make "Try again" the link.

The text in the Output screen felt a little too long for me. How about we go back to using an intro and tooltip:

Intro: View the final output data.
Tooltip: See how each processor affects the document as it passes through the pipeline.

cjcenizal · 2020-05-05T16:52:52Z

Unable to load pipelines. Try again.

RGTM!

The text in the Output screen felt a little too long for me. How about we go back to using an intro and tooltip:

I'm not sure the intro/tooltip option is equivalent with what we currently have. The current text offers the user two exclusive options: final output data or data per-processor. But your suggestion makes it sound like the tooltip text is an elaboration upon the intro text.

cchaos · 2020-05-05T16:54:37Z

Hey @alisonelizabeth I just glanced at your screenshots and have one request. Can you separate create a single empty state panel instead of putting the empty state underneath the title for the listing page? It just creates a weird hierarchy as it is and then you can explain more of what a pipeline is and duplicate the docs link inside of it.

alisonelizabeth · 2020-05-05T16:57:35Z

@cchaos thanks for the catch! ~~I will open up a separate PR and address this.~~ I went ahead and addressed in this PR.

gchaps · 2020-05-05T17:45:24Z

Empty state

How about using "Learn more" followed by an icon to let users know they are going to an outside doc.

Output

I'm not sure the intro/tooltip option is equivalent with what we currently have.

We could keep the intro (I removed a few words) and then use a space to separate the intro from the Refresh link so it doesn't feel so crowded)

View the output data, or see how each processor affects the document as it passes through the pipeline.

Failure processors

What about adding the word "alternate". Some choices:

The alternate processors to execute after a processor fails.
The alternate processors to execute after the pipeline stops because of a failure.
The alternate processors to execute after an original processor in the pipeline fails.

…nes/copy

alisonelizabeth · 2020-05-05T20:12:14Z

@gchaps @cjcenizal I believe I addressed all of your feedback. Thanks for the review! Updated screenshots:

Empty prompt

Output tab - I'm not sure if I'm sold on the "Refresh output" link on its own line. @cchaos WDYT?

Create form

Error

cchaos · 2020-05-05T21:36:31Z

Thanks @alisonelizabeth for going ahead and tackling the empty screen! Could you also add a small line break before the "Learn more" link? Just to ensure that there isn't a line break inside of the link itself.

As for the refresh button. AFAICT, It refreshes the code block below? If it's crammed up with the description text above then it seems more like a link than a button. I would even go further and maybe make it a proper button (the borderd, but small kind).

You could also put the refresh button and the toggle in the same line with the toggle on the far right. So then it looks like a toolbar for the code output.

gchaps · 2020-05-05T21:45:57Z

Text looks good. I have one small comment. For the error message, can we use "Try again"?

alisonelizabeth · 2020-05-06T01:10:06Z

Text looks good. I have one small comment. For the error message, can we use "Try again"?

@gchaps oops! The screenshot I added was outdated. The text is "Try again" now.

alisonelizabeth · 2020-05-06T01:12:38Z

Thanks @cchaos! I added a line break to the empty prompt and also applied your suggestion regarding the "Reload" functionality. I think this looks a lot better now - thanks!

kibanamachine · 2020-05-06T02:58:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 569a4f4

History

💔 Build #45701 failed fbcfb7c
💚 Build #45458 succeeded 0511a3c
💔 Build #45359 failed 8454ef2
💚 Build #45254 succeeded 5ef63d8c544b92bbf6943e40807921f0b4d2a558

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

cjcenizal · 2020-05-06T03:12:32Z

I’m going to merge this so we make FF.

* master: (72 commits) add tsvb tests to Firefox suite (elastic#65425) Fix flaky ServerMetricsCollector integration test (elastic#65420) [APM] Custom links section inside the Actions menu is showing outside of the menu (elastic#65428) [ML] Adds docs_per_second to transform edit form. (elastic#65365) update apm index pattern (elastic#65424) add direct build command (elastic#65431) [ML] Adding daily_model_snapshot_retention_after_days to types and schemas (elastic#65417) [chore] Improve request cancelation handling in vis embeddable (elastic#65057) [Alerting] migrates acceptance and functional test fixtures to KP (elastic#64888) [ML] Fixes reordering in view by selection when overall cell selected (elastic#65290) Additional branding updates (elastic#64712) Remove redundant formatting of percentage column (elastic#64948) [SIEM][CASE] Configuration pages UI redesign (elastic#65355) New nav (elastic#64018) [Ingest pipelines] Address copy feedback (elastic#65175) bug fixing (elastic#65387) skip whole suite blocking snapshots (elastic#65377) add related event generation to ancestor nodes (fixes a bug) (elastic#64950) [Canvas] move files from legacy/plugins to plugins (elastic#65283) [SIEM] template timeline UI (elastic#64439) ...

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

address copy feedback

4878cde

alisonelizabeth force-pushed the ingest_pipelines/copy branch from 5ef63d8 to 4878cde Compare May 5, 2020 01:31

alisonelizabeth marked this pull request as ready for review May 5, 2020 01:34

alisonelizabeth requested a review from a team as a code owner May 5, 2020 01:34

alisonelizabeth requested review from gchaps and jloleysens May 5, 2020 01:34

alisonelizabeth commented May 5, 2020

View reviewed changes

alisonelizabeth added 2 commits May 4, 2020 21:36

Merge branch 'master' of github.com:elastic/kibana into ingest_pipeli…

8454ef2

…nes/copy

remove unused import

0511a3c

cjcenizal approved these changes May 5, 2020

View reviewed changes

alisonelizabeth removed the request for review from jloleysens May 5, 2020 16:34

alisonelizabeth added 2 commits May 5, 2020 15:42

Merge branch 'master' of github.com:elastic/kibana into ingest_pipeli…

72d59a0

…nes/copy

address feedback

fbcfb7c

address design feedback

569a4f4

cjcenizal merged commit 9d88805 into elastic:master May 6, 2020

cjcenizal mentioned this pull request May 6, 2020

[7.x] [Ingest pipelines] Address copy feedback (#65175) #65410

Merged

cjcenizal pushed a commit to cjcenizal/kibana that referenced this pull request May 6, 2020

[Ingest pipelines] Address copy feedback (elastic#65175)

3aba706

cjcenizal added a commit that referenced this pull request May 6, 2020

[7.x] [Ingest pipelines] Address copy feedback (#65175) (#65410)

211d4c4

[Ingest pipelines] Address copy feedback #65175

[Ingest pipelines] Address copy feedback #65175

Uh oh!

Conversation

alisonelizabeth commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

Uh oh!

elasticmachine commented May 4, 2020

Uh oh!

alisonelizabeth May 5, 2020

Choose a reason for hiding this comment

Uh oh!

cjcenizal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjcenizal May 5, 2020

Choose a reason for hiding this comment

Uh oh!

alisonelizabeth May 5, 2020

Choose a reason for hiding this comment

Uh oh!

cjcenizal May 5, 2020

Choose a reason for hiding this comment

Uh oh!

alisonelizabeth May 5, 2020

Choose a reason for hiding this comment

Uh oh!

gchaps commented May 5, 2020

Uh oh!

cjcenizal commented May 5, 2020

Uh oh!

cchaos commented May 5, 2020

Uh oh!

alisonelizabeth commented May 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gchaps commented May 5, 2020

Uh oh!

alisonelizabeth commented May 5, 2020

Uh oh!

cchaos commented May 5, 2020

Uh oh!

gchaps commented May 5, 2020

Uh oh!

alisonelizabeth commented May 6, 2020

Uh oh!

alisonelizabeth commented May 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented May 6, 2020

💚 Build Succeeded

History

Uh oh!

cjcenizal commented May 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alisonelizabeth commented May 4, 2020 •

edited

Loading

cjcenizal left a comment •

edited

Loading

alisonelizabeth commented May 5, 2020 •

edited

Loading

alisonelizabeth commented May 6, 2020 •

edited

Loading