UI improvements to watch list page #36816

alisonelizabeth · 2019-05-21T18:16:34Z

This PR makes some UI improvements to the watch list page

Added docs link
Added empty state
Tweaked the width for some of the columns to give some more room to the ID column
Changed the location of the buttons to better align with other management apps
Added delete option to actions column
Updated delete text toast notification (Related: [watcherUI] The "count" of watches deleted can be confusing to the user. #17956)

Screenshots

Before:

Latest changes:

alisonelizabeth · 2019-05-21T18:18:45Z

x-pack/plugins/watcher/public/sections/watch_list/components/watch_list.tsx

+      </EuiFlexItem>
+    </EuiFlexGroup>
+  );
+
  if (getPageErrorCode(error)) {


I think this is only checking for 403 and 404 errors. Do we need to guard against other status codes?

Yes I think it's probably best to render a danger callout with the error status and message in the event of non-403 and non-404 errors, since if there's an error then the table can't render content and might throw a fatal error (e.g. by trying to access a property on an undefined variable). I think a callout is better than a toast notification in this case because a toast notification is transient and the user might want to report the error in an issue or something. WDYT?

yeah, makes sense - updated.

elasticmachine · 2019-05-21T18:22:23Z

Pinging @elastic/es-ui

elasticmachine · 2019-05-21T19:32:33Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cjcenizal

Great work tracking down all of these details! They really add up. Just had a few comments and suggestions.

cjcenizal · 2019-05-23T03:16:47Z

x-pack/plugins/watcher/public/components/delete_watches_modal.tsx

@@ -58,8 +58,8 @@ export const DeleteWatchesModal = ({
                'xpack.watcher.sections.watchList.deleteSelectedWatchesSuccessNotification.descriptionText',
                {
                  defaultMessage:
-                    'Deleted {numWatchesToDelete, plural, one {watch} other {{numSuccesses} out of # selected watches}} ',
-                  values: { numSuccesses, numWatchesToDelete },
+                    'Successfuly deleted {numSuccesses, number} {numSuccesses, plural, one {watch} other {watches}}',


I think we can drop "Successfully". I know we use this in Index Management but I don't see it anywhere else. Feel free to confirm this with Gail.

cjcenizal · 2019-05-23T03:17:37Z

x-pack/plugins/watcher/public/lib/documentation_links/documentation_links.ts

@@ -12,6 +12,7 @@ const esStackBase = `${ELASTIC_WEBSITE_URL}guide/en/elastic-stack-overview/${DOC

 export const putWatchApiUrl = `${esBase}/watcher-api-put-watch.html`;
 export const executeWatchApiUrl = `${esBase}/watcher-api-execute-watch.html#watcher-api-execute-watch-action-mode`;
+export const watcherGettingStartedUrl = `${esStackBase}/watcher-getting-started.html`;


I think it might be more helpful to link to the Watcher UI docs: https://www.elastic.co/guide/en/kibana/current/watcher-ui.html

In fact, we should update the screenshots in the docs as part of this work. Would you mind adding it as an item to the roadmap?

cjcenizal · 2019-05-23T03:20:53Z

x-pack/plugins/watcher/public/sections/watch_list/components/watch_list.tsx

+        render: (id: string) => {
+          return (
+            <EuiLink
+              className="indTable__link euiTableCellContent"


Is this class name left over from a copy/paste? indTable__link

yep, looks that way. deleted the class.

cjcenizal · 2019-05-23T03:22:46Z

x-pack/plugins/watcher/public/sections/watch_list/components/watch_list.tsx

+                    aria-label={i18n.translate(
+                      'xpack.watcher.sections.watchList.watchTable.actionEditAriaLabel',
+                      {
+                        defaultMessage: 'Edit watch `{name}`',


Can we change this to:

`Edit watch '{name}'`

I'm not sure backticks work as quotation marks in the original code. Perhaps worth checking with Gail.

good catch, fixed!

cjcenizal · 2019-05-23T03:23:07Z

x-pack/plugins/watcher/public/sections/watch_list/components/watch_list.tsx

+                    aria-label={i18n.translate(
+                      'xpack.watcher.sections.watchList.watchTable.actionDeleteAriaLabel',
+                      {
+                        defaultMessage: 'Delete watch `{name}`',


cjcenizal · 2019-05-23T03:27:19Z

x-pack/plugins/watcher/public/sections/watch_list/components/watch_list.tsx

@@ -52,6 +55,56 @@ const WatchListUi = ({ intl }: { intl: InjectedIntl }) => {
    [watches, deletedWatches]
  );

+  const createWatchButtons = (
+    <EuiFlexGroup gutterSize="m">


We need to apply justifyContent="center" to this flex group when the buttons are presented in the empty state in order to align them along the vertical axis with the other content:

I'm not sure if this prop will make sense when the buttons are used elsewhere, so maybe we'll have to wrap them in a function that lets you conditionally apply this prop.

good catch! i didn't see any adverse affects with having justifyContent="center" applied when the buttons are aligned with the search so i did not add any conditionally logic.

cjcenizal · 2019-05-23T03:38:01Z

x-pack/plugins/watcher/public/sections/watch_list/components/watch_list.tsx

+      </EuiFlexItem>
+    </EuiFlexGroup>
+  );
+
  if (getPageErrorCode(error)) {


Yes I think it's probably best to render a danger callout with the error status and message in the event of non-403 and non-404 errors, since if there's an error then the table can't render content and might throw a fatal error (e.g. by trying to access a property on an undefined variable). I think a callout is better than a toast notification in this case because a toast notification is transient and the user might want to report the error in an issue or something. WDYT?

alisonelizabeth · 2019-05-23T14:14:20Z

@cjcenizal thanks for your review! I have made the changes you suggested.

elasticmachine · 2019-05-23T15:26:14Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cjcenizal

LGTM!

alisonelizabeth added Feature:Watcher non-issue Indicates to automation that a pull request should not appear in the release notes labels May 21, 2019

alisonelizabeth commented May 21, 2019

View reviewed changes

alisonelizabeth added the Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more label May 21, 2019

alisonelizabeth requested a review from cjcenizal May 21, 2019 18:22

cjcenizal suggested changes May 23, 2019

View reviewed changes

alisonelizabeth added 3 commits May 23, 2019 10:05

UI improvements to watch list page

caf65b5

i18n fix

ea4e6d9

address review feedback

fc27f21

alisonelizabeth force-pushed the watch-list-improvements branch from cc808d5 to fc27f21 Compare May 23, 2019 14:10

cjcenizal approved these changes May 23, 2019

View reviewed changes

alisonelizabeth merged commit 24802ae into elastic:watcher-port May 23, 2019

alisonelizabeth deleted the watch-list-improvements branch May 23, 2019 22:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI improvements to watch list page #36816

UI improvements to watch list page #36816

alisonelizabeth commented May 21, 2019 •

edited

Loading

alisonelizabeth May 21, 2019

cjcenizal May 23, 2019

alisonelizabeth May 23, 2019

elasticmachine commented May 21, 2019

elasticmachine commented May 21, 2019

cjcenizal left a comment

cjcenizal May 23, 2019

cjcenizal May 23, 2019

alisonelizabeth May 23, 2019

cjcenizal May 23, 2019

alisonelizabeth May 23, 2019

cjcenizal May 23, 2019

alisonelizabeth May 23, 2019

cjcenizal May 23, 2019

alisonelizabeth May 23, 2019

cjcenizal May 23, 2019

alisonelizabeth May 23, 2019

cjcenizal May 23, 2019

alisonelizabeth commented May 23, 2019

elasticmachine commented May 23, 2019

cjcenizal left a comment

UI improvements to watch list page #36816

UI improvements to watch list page #36816

Conversation

alisonelizabeth commented May 21, 2019 • edited Loading

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 21, 2019

elasticmachine commented May 21, 2019

💔 Build Failed

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alisonelizabeth commented May 23, 2019

elasticmachine commented May 23, 2019

💚 Build Succeeded

cjcenizal left a comment

Choose a reason for hiding this comment

alisonelizabeth commented May 21, 2019 •

edited

Loading