[ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. #70555

walterra · 2020-07-02T07:02:58Z

Refactors the action buttons of the transform and data frame analytics jobs list:

Previously custom actions included state and JSX for e.g. confirmation modals. Problem with that: If the actions list popover hides, the modal would unmount too. Since EUI's behaviour will change with the release/merge of #70383, we needed a refactor that solves that issue right now.

With this PR, state management for UI behaviour that follows after a button click like the confirmation modals was moved to a custom hook which is part of the outer level of the buttons itself. The modal now also gets mounted on the outer level. This way we won't loose the modals state and DOM rendering when the action button hides.

Note that this PR doesn't fix the nested buttons issue (#63455) yet. For that we need EUI issue #70383 to be in Kibana which will arrive with EUI v26.3.0 via #70243. So there will be one follow up to that which will focus on getting rid of the nested button structure.

Todos

Transform actions
Data Frame Analytics actions
Re-Merge and refactor DFA edit functionality ([ML] DF Analytics: add ability to edit job for fields supported by API #70489) once available in master

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…n click.

elasticmachine · 2020-07-03T10:07:53Z

Pinging @elastic/ml-ui (:ml)

walterra · 2020-07-03T10:12:28Z

@thompsongl This PR resolves #70383 in preparation for the EUI update in #70243. No need for you to review the full PR, added you as a reviewer for visibility.

darnautov · 2020-07-03T14:42:49Z

..._frame_analytics/pages/analytics_management/components/action_delete/delete_button_modal.tsx

+
+  return (
+    <>
+      {isModalVisible && (


I guess it would be cleaner if a parent component will be responsible for hiding the modal

Updated in 4382fee

peteharverson · 2020-07-03T14:32:38Z

...n/data_frame_analytics/pages/analytics_management/components/action_delete/delete_button.tsx

+            ? i18n.translate(
+                'xpack.ml.dataframe.analyticsList.deleteActionDisabledToolTipContent',
+                {
+                  defaultMessage: 'Stop the data frame analytics in order to delete it.',


I think this would read better with the word job in it - Stop the data frame analytics job in order to delete it.

Updated in f4186e7.

...ta_frame_analytics/pages/analytics_management/components/action_start/start_button_modal.tsx

darnautov · 2020-07-03T14:47:48Z

...ation/data_frame_analytics/pages/analytics_management/components/action_view/view_button.tsx

+      })}
+      data-test-subj="mlAnalyticsJobViewButton"
+    >
+      {i18n.translate('xpack.ml.dataframe.analyticsList.viewActionName', {


nit: can be replaced with FormattedMessage

...on/data_frame_analytics/pages/analytics_management/components/analytics_list/use_actions.tsx

...ion/data_frame_analytics/pages/analytics_management/components/action_clone/clone_button.tsx

peteharverson

Tested and LGTM

walterra · 2020-07-07T10:21:14Z

@alvarezmelissa87 I re-merged master into this PR and refactored the recently introduced editing feature for DFA to match the updated code structure introduced in this PR. Also added you as a reviewer since you did the editing feature.

kibanamachine · 2020-07-07T13:36:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5de8ec3

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
ml	386	-2	388

History

💔 Build #59315 failed 1c42f67
💔 Build #58889 failed e47755b
💔 Build #58867 failed 0d299e9
💔 Build #58632 failed 996e151
💔 Build #58334 failed dd42b8138b1942aec66c6bc3312ad7d8bae5cf6d

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

walterra · 2020-07-07T13:43:50Z

@darnautov addressed your comments, and all tests finally passed 🥳 please have another look 👁️

alvarezmelissa87

Gave this a test - LGTM ⚡

darnautov

LGTM

…ount after button click. (elastic#70555) Related to elastic#70383 and elastic#63455. Refactors the action buttons of the transform and data frame analytics jobs list: Previously custom actions included state and JSX for e.g. confirmation modals. Problem with that: If the actions list popover hides, the modal would unmount too. Since EUI's behaviour will change with the release/merge of elastic#70383, we needed a refactor that solves that issue right now. With this PR, state management for UI behaviour that follows after a button click like the confirmation modals was moved to a custom hook which is part of the outer level of the buttons itself. The modal now also gets mounted on the outer level. This way we won't loose the modals state and DOM rendering when the action button hides. Note that this PR doesn't fix the nested buttons issue (elastic#63455) yet. For that we need EUI issue elastic#70383 to be in Kibana which will arrive with EUI v26.3.0 via elastic#70243. So there will be one follow up to that which will focus on getting rid of the nested button structure.

…ount after button click. (#70555) (#71050) Related to #70383 and #63455. Refactors the action buttons of the transform and data frame analytics jobs list: Previously custom actions included state and JSX for e.g. confirmation modals. Problem with that: If the actions list popover hides, the modal would unmount too. Since EUI's behaviour will change with the release/merge of #70383, we needed a refactor that solves that issue right now. With this PR, state management for UI behaviour that follows after a button click like the confirmation modals was moved to a custom hook which is part of the outer level of the buttons itself. The modal now also gets mounted on the outer level. This way we won't loose the modals state and DOM rendering when the action button hides. Note that this PR doesn't fix the nested buttons issue (#63455) yet. For that we need EUI issue #70383 to be in Kibana which will arrive with EUI v26.3.0 via #70243. So there will be one follow up to that which will focus on getting rid of the nested button structure. Co-authored-by: Elastic Machine <[email protected]>

* master: [RUM Dashboard] New rum services api to replace usage of get services API (elastic#70746) fix: remove only consecutive ticks in TSVB (elastic#70981) [Functional test] Increase the timeout on opening a saved visualization (elastic#70952) [ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. (elastic#70555) [Functional test] Add retry for dashboard save (elastic#70950)

walterra added :ml Feature:Transforms ML transforms Feature:Data Frame Analytics ML data frame analytics features labels Jul 2, 2020

walterra self-assigned this Jul 2, 2020

walterra force-pushed the ml-fix-list-action-buttons branch from 9a98e59 to 9033dbf Compare July 2, 2020 14:41

walterra added 5 commits July 3, 2020 11:59

[ML] Refactor list action buttons so modals won't unmount after butto…

00d2955

…n click.

[ML] Update tests.

001fafd

[ML] Rename hook files.

1b72310

[ML] Updated file structure.

bb91e7d

[ML] Refactor analytics list actions.

996e151

walterra force-pushed the ml-fix-list-action-buttons branch from dd42b81 to 996e151 Compare July 3, 2020 09:59

walterra added refactoring release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels Jul 3, 2020

walterra requested review from darnautov, jgowdyelastic and peteharverson July 3, 2020 10:03

walterra marked this pull request as ready for review July 3, 2020 10:07

walterra requested a review from a team as a code owner July 3, 2020 10:07

walterra requested a review from thompsongl July 3, 2020 10:10

darnautov reviewed Jul 3, 2020

View reviewed changes

peteharverson reviewed Jul 3, 2020

View reviewed changes

darnautov reviewed Jul 3, 2020

View reviewed changes

...ta_frame_analytics/pages/analytics_management/components/action_start/start_button_modal.tsx Outdated Show resolved Hide resolved

darnautov reviewed Jul 3, 2020

View reviewed changes

...on/data_frame_analytics/pages/analytics_management/components/analytics_list/use_actions.tsx Outdated Show resolved Hide resolved

...ion/data_frame_analytics/pages/analytics_management/components/action_clone/clone_button.tsx Show resolved Hide resolved

peteharverson approved these changes Jul 3, 2020

View reviewed changes

[ML] Text tweak.

f4186e7

walterra added 2 commits July 6, 2020 11:47

[ML] Simplify props.

4382fee

Merge branch 'master' into ml-fix-list-action-buttons

0d299e9

walterra requested a review from darnautov July 6, 2020 09:53

walterra added 5 commits July 6, 2020 13:10

[ML] Fix imports and types.

e47755b

Merge branch 'master' into ml-fix-list-action-buttons

c681bfe

[ML] Update tests to reflect new required component structure.

d7cf579

Merge branch 'master' into ml-fix-list-action-buttons

13954f7

[ML] Update tests to reflect new required component structure.

1c42f67

walterra requested a review from alvarezmelissa87 July 7, 2020 10:20

[ML] Fix transform edit button setup.

5de8ec3

alvarezmelissa87 approved these changes Jul 7, 2020

View reviewed changes

darnautov approved these changes Jul 8, 2020

View reviewed changes

walterra merged commit bb96f5d into elastic:master Jul 8, 2020

walterra deleted the ml-fix-list-action-buttons branch July 8, 2020 08:10

walterra mentioned this pull request Jul 8, 2020

[7.x] [ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. (#70555) #71050

Merged

walterra mentioned this pull request Jul 8, 2020

Upgrade EUI to v26.3.1 #70243

Merged

walterra mentioned this pull request Jul 14, 2020

[ML] Invalid actions pattern on Transforms management table #70383

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. #70555

[ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. #70555

walterra commented Jul 2, 2020 •

edited

Loading

elasticmachine commented Jul 3, 2020

walterra commented Jul 3, 2020

darnautov Jul 3, 2020

walterra Jul 6, 2020 •

edited

Loading

peteharverson Jul 3, 2020

walterra Jul 6, 2020

darnautov Jul 3, 2020

peteharverson left a comment

walterra commented Jul 7, 2020 •

edited

Loading

kibanamachine commented Jul 7, 2020

walterra commented Jul 7, 2020

alvarezmelissa87 left a comment

darnautov left a comment

[ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. #70555

[ML] Transforms/DFA: Refactor list action buttons so modals won't unmount after button click. #70555

Conversation

walterra commented Jul 2, 2020 • edited Loading

Todos

Checklist

For maintainers

elasticmachine commented Jul 3, 2020

walterra commented Jul 3, 2020

darnautov Jul 3, 2020

Choose a reason for hiding this comment

walterra Jul 6, 2020 • edited Loading

Choose a reason for hiding this comment

peteharverson Jul 3, 2020

Choose a reason for hiding this comment

walterra Jul 6, 2020

Choose a reason for hiding this comment

darnautov Jul 3, 2020

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

walterra commented Jul 7, 2020 • edited Loading

kibanamachine commented Jul 7, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

History

walterra commented Jul 7, 2020

alvarezmelissa87 left a comment

Choose a reason for hiding this comment

darnautov left a comment

Choose a reason for hiding this comment

walterra commented Jul 2, 2020 •

edited

Loading

walterra Jul 6, 2020 •

edited

Loading

walterra commented Jul 7, 2020 •

edited

Loading