Watcher history page: port to react #33047

pcsanwald · 2019-03-12T20:43:21Z

This is a work in progress PR that I'm opening to get feedback on the implementation.

Here's an april 1st walkthrough:

So far, I set up two EuiInMemoryTables alongside the existing tables, and have changed over the detail page from a separate route, over to an EuiFlyout.

Incomplete list of things left to do:

Populate flyout page with correct data (Execution Status and Execution Output)
Add actions to main table to deactivate watch
Support old watch history detail page links by supporting optional URL
Add unit tests
Design review
Check if a watch is disabled, if so, toggle default disable button state

elasticmachine · 2019-03-12T21:34:00Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

bmcconaghy · 2019-03-13T19:56:20Z

@pcsanwald this is looking good.

elasticmachine · 2019-03-13T22:15:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-14T20:46:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-20T00:32:16Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-22T14:40:34Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-25T23:05:54Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-25T23:39:03Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-29T20:14:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-01T21:43:57Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-02T01:47:35Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

yaronp68 · 2019-04-02T08:56:33Z

@pcsanwald The date time format is not friendly IMO. I guess having the same format as the EUI date picker makes sense

gchaps

@pcsanwald Here are some suggested edits.

Watch History Detail flyout

Use sentence case for titles:

Watch history detail

Delete watch modal

Suggest including watch name in title (if they are not tool long) and removing "Are you sure."

Delete watch watch_name

You can’t recover a deleted watch.

Cancel | Delete watch

Current Status page

Sentence case for titles:

Current status
Watch history

Sentence case for button labels:

Activate watch
Deactivate watch

Shorten empty state text:

No current status to show -> No current status

Toast on deletion

If possible, include watch name:

Watch watch_name deleted.

Otherwise:

Watch deleted.

It would be nice to be more specific here. The reason I got this message was that the watch was already deleted.

pcsanwald · 2019-04-03T20:19:40Z

thanks for the feedback @gchaps, will work on all these things

alisonelizabeth · 2019-04-04T13:04:26Z

...plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_action_status.tsx

@@ -0,0 +1,25 @@
+/*


@pcsanwald I think this is something that I'd like to reuse for the advanced watcher pages too. Maybe we should move it under /watcher/public/components?

Generally speaking, I'm not sure how we want to approach the file structure. However, right now, it seems we have some common components in there already (e.g., form_errors.tsx, delete_watches_modal.tsx) FYI @bmcconaghy

yeah seems like a good candidate for moving up to the common components dir.

should we do this after merge, or should I go ahead and do in this PR? I'm fine either way

another PR is fine I think

elasticmachine · 2019-04-09T18:01:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-10T13:47:39Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

bmcconaghy

Looking good, just left a few minor comments. This will need some tests, but those can be added in a follow up PR

bmcconaghy · 2019-04-10T14:04:28Z

...plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_action_status.tsx

@@ -0,0 +1,25 @@
+/*


yeah seems like a good candidate for moving up to the common components dir.

bmcconaghy · 2019-04-10T14:05:55Z

...ins/watcher/public/sections/watch_detail/components/watch_detail/watch_history_component.tsx

+  }>({});
+  const [executionDetail, setExecutionDetail] = useState<string>('');
+
+  const kbnUrlService = urlService;


not sure why this is done

addressed in 2b23125

bmcconaghy · 2019-04-10T14:06:20Z

...ins/watcher/public/sections/watch_detail/components/watch_detail/watch_history_component.tsx

+  };
+
+  const watchHistoryTimeSpanOptions = [
+    { value: 'now-1h', text: 'Last one hour' },


these probably need i18n

addressed in 2b23125

elasticmachine · 2019-04-10T17:56:25Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-11T11:43:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-11T16:36:43Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

bmcconaghy

LGTM, just a couple of minor comments.

bmcconaghy · 2019-04-11T16:41:39Z

...ins/watcher/public/sections/watch_detail/components/watch_detail/watch_history_component.tsx

+    } else {
+      await activateWatch(watchId);
+    }
+    // TODO[pcs]: error handling, response checking, etc...


is this still a valid TODO?

bmcconaghy · 2019-04-11T16:42:58Z

...ins/watcher/public/sections/watch_detail/components/watch_detail/watch_history_component.tsx

+    },
+  ];
+
+  const onTimespanChange = (e: any) => {


this should work (and it will be more specific): (e: React.ChangeEvent<HTMLInputElement>)

bmcconaghy · 2019-04-11T16:43:04Z

...ins/watcher/public/sections/watch_detail/components/watch_detail/watch_history_component.tsx

+    },
+  ];
+
+  const onTimespanChange = (e: any) => {


this should work (and it will be more specific): (e: React.ChangeEvent<HTMLInputElement>)

bmcconaghy · 2019-04-11T16:43:09Z

...ins/watcher/public/sections/watch_detail/components/watch_detail/watch_history_component.tsx

+    },
+  ];
+
+  const onTimespanChange = (e: any) => {


this should work (and it will be more specific): (e: React.ChangeEvent<HTMLInputElement>)

bmcconaghy · 2019-04-11T16:43:12Z

...ins/watcher/public/sections/watch_detail/components/watch_detail/watch_history_component.tsx

+    },
+  ];
+
+  const onTimespanChange = (e: any) => {


this should work (and it will be more specific): (e: React.ChangeEvent<HTMLInputElement>)

addressed in 7e6c10a

elasticmachine · 2019-04-11T20:47:35Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-11T23:04:27Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-11T23:46:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

Paul Sanwald added 7 commits March 12, 2019 16:35

get react component rendering inside of existing angular watcher page

06d36ec

populate status table with basic data

7a62543

add watch history component, populate table with data

3b7e87f

format dates and statuses (statii?) better

af4fdc1

fix checkstyle

4821892

set up basic flyout panel

8c6d422

get data appearing on flyout page

a7a0bca

pcsanwald requested a review from bmcconaghy March 12, 2019 20:43

get action statii appearing on flyout

80cd29f

get execution detail to appear on flyout

df074e2

get deactivate basic functionality working

d71afb9

get delete button working

6cd819a

Paul Sanwald added 2 commits March 25, 2019 16:00

add time picker to UI

b21928b

remove outdated todos, fix bug in flyout detail

b466ee0

refactor state mgmt into its own component

2f1e2fa

fix layout of tables

6863ed2

add watch layout

a5f6de1

pcsanwald requested a review from gchaps April 1, 2019 22:52

gchaps reviewed Apr 3, 2019

View reviewed changes

alisonelizabeth reviewed Apr 4, 2019

View reviewed changes

merge from base branch, address conflicts

c7a8cc9

add sentence casing, remove unnecessary import

2f7ceb8

bmcconaghy approved these changes Apr 10, 2019

View reviewed changes

address code review comments

2b23125

Paul Sanwald added 2 commits April 10, 2019 16:10

Merge branch 'watcher-port' into watch-history-port

1f5ff2c

disambiguate i18n keys for messages

4e0bab2

correct linting errors

9d65367

bmcconaghy approved these changes Apr 11, 2019

View reviewed changes

Paul Sanwald added 2 commits April 11, 2019 14:45

add more specific type

a147593

add update of activation status based on status

7e6c10a

address code review comments

073db16

i18n disambiguation

5bb3500

pcsanwald merged commit 0078417 into elastic:watcher-port Apr 12, 2019

Watcher history page: port to react #33047

Watcher history page: port to react #33047

Conversation

pcsanwald commented Mar 12, 2019 • edited Loading

elasticmachine commented Mar 12, 2019

💔 Build Failed

bmcconaghy commented Mar 13, 2019

elasticmachine commented Mar 13, 2019

💔 Build Failed

elasticmachine commented Mar 14, 2019

💔 Build Failed

elasticmachine commented Mar 20, 2019

💔 Build Failed

elasticmachine commented Mar 22, 2019

💔 Build Failed

elasticmachine commented Mar 25, 2019

💔 Build Failed

elasticmachine commented Mar 25, 2019

💔 Build Failed

elasticmachine commented Mar 29, 2019

💔 Build Failed

elasticmachine commented Apr 1, 2019

💔 Build Failed

elasticmachine commented Apr 2, 2019

💔 Build Failed

yaronp68 commented Apr 2, 2019 • edited Loading

gchaps left a comment

Choose a reason for hiding this comment

Watch History Detail flyout

Delete watch modal

Current Status page

Toast on deletion

pcsanwald commented Apr 3, 2019

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

elasticmachine commented Apr 9, 2019

💔 Build Failed

elasticmachine commented Apr 10, 2019

💔 Build Failed

bmcconaghy 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

elasticmachine commented Apr 10, 2019

💔 Build Failed

elasticmachine commented Apr 11, 2019

💔 Build Failed

elasticmachine commented Apr 11, 2019

💚 Build Succeeded

bmcconaghy 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

elasticmachine commented Apr 11, 2019

💔 Build Failed

elasticmachine commented Apr 11, 2019

💔 Build Failed

elasticmachine commented Apr 11, 2019

💚 Build Succeeded

pcsanwald commented Mar 12, 2019 •

edited

Loading

yaronp68 commented Apr 2, 2019 •

edited

Loading