[ML] calendar eui conversion #26741

alvarezmelissa87 · 2018-12-05T23:36:20Z

Summary

Settings -> Calendar management (settings/scheduled_events/) rewrite to React/eui (settings/calendar/).

Meta issue with details and screenshots: #25970

Note:

Adding tests. File structure will change as I add them.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
- IE11 issue to be fixed in follow up PR
  ~~- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
  ~~- [ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~

This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2018-12-05T23:36:21Z

Pinging @elastic/ml-ui

elasticmachine · 2018-12-06T00:23:12Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

x-pack/plugins/ml/public/settings/_index.scss

snide

I'd start using the EUI name schema on new sass files. I know we didn't convert ML completely to this yet, but it'll save you trouble when we do make those changes.

x-pack/plugins/ml/public/settings/calendar/_calendar.scss

x-pack/plugins/ml/public/settings/calendar/edit/_edit.scss

x-pack/plugins/ml/public/settings/calendar/_calendar.scss

x-pack/plugins/ml/public/settings/calendar/edit/_edit.scss

x-pack/plugins/ml/public/settings/calendar/edit/directive.js

x-pack/plugins/ml/public/settings/calendar/edit/import_modal.js

x-pack/plugins/ml/public/settings/calendar/edit/new_calendar.js

x-pack/plugins/ml/public/settings/calendar/edit/utils.js

x-pack/plugins/ml/public/settings/calendar/list/delete_calendars.js

x-pack/plugins/ml/public/settings/calendar/list/directive.js

elasticmachine · 2018-12-06T23:01:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-12-07T18:15:16Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-12-07T20:07:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

x-pack/plugins/ml/public/settings/calendar/edit/calendar_form.js

x-pack/plugins/ml/public/settings/calendar/list/calendars_list.js

x-pack/plugins/ml/public/settings/calendar/edit/new_calendar.js

x-pack/plugins/ml/public/settings/calendar/list/table/table.js

x-pack/plugins/ml/public/settings/calendar/edit/new_calendar.js

x-pack/plugins/ml/public/settings/calendar/edit/directive.js

peteharverson · 2018-12-10T13:17:49Z

x-pack/plugins/ml/public/settings/calendar/edit/directive.js

+
+uiRoutes
+  .when('/settings/calendars_list/new_calendar', {
+    template,


You'll need to pull in the k7breadcrumbs from x-pack/plugins/ml/public/settings/breadcrumbs.js by rebasing to latest master

👍 Doing this in a follow up PR to avoid backport messiness since the breadcrumbs update is only in master. This way I can backport this PR and just leave the route update follow-up PR in master. 😄

peteharverson · 2018-12-10T13:18:17Z

x-pack/plugins/ml/public/settings/calendar/edit/directive.js

+    }
+  })
+  .when('/settings/calendars_list/edit_calendar/:calendarId', {
+    template,


As above, you'll need to pull in the k7breadcrumbs from x-pack/plugins/ml/public/settings/breadcrumbs.js by rebasing

Same as above comment. 👍

x-pack/plugins/ml/public/settings/calendar/edit/new_calendar.js

peteharverson · 2018-12-10T13:29:08Z

x-pack/plugins/ml/public/settings/calendar/edit/new_event_modal.js

+        {timeInputs}
+        <EuiSpacer size="s" />
+        <EuiFormRow fullWidth>
+          <EuiDatePickerRange


Is this component fully accessible as used here? I wasn't able to navigate to particular dates

All the inputs and the arrows to move between months are - this should give keyboard access to all functionality. Moving around through each day in the calendars themselves looks like it isn't - not sure we can override that on our side, though. 🤔

x-pack/plugins/ml/public/settings/calendar/list/calendars_list.js

x-pack/plugins/ml/public/settings/calendar/list/directive.js

elasticmachine · 2018-12-10T16:02:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-12-10T17:26:56Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-12-10T18:08:14Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-12-10T19:18:10Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alvarezmelissa87 · 2018-12-10T19:18:48Z

I've updated with all suggested changes if you wanted to take another look when you get a chance. 😄
cc @peteharverson, @jgowdyelastic

elasticmachine · 2018-12-11T00:07:22Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

peteharverson · 2018-12-11T10:40:21Z

x-pack/plugins/ml/public/settings/calendars/edit/new_calendar.js

+
+  onEventDelete = (eventId) => {
+    this.setState(prevState => ({
+      events: prevState.events.filter(event => event.event_id !== eventId)


I think there is a problem with this for events which have been added, but before the calendar has been saved. Looks like events only pick up an ID after saving, so if you delete a 'new' event (as in one which has yet to be saved), all 'new' events end up being deleted.

Heya @peteharverson - this is fixed now. The temp event id wasn't being passed on to the new_calendar form so there was no id to keep track of deletions. 👍

peteharverson

A few styling edits are needed!

x-pack/plugins/ml/public/settings/_index.scss

x-pack/plugins/ml/public/settings/calendars/_calendar.scss

walterra · 2018-12-11T11:20:41Z

I saw you have used css classes like .mlCalendar_list or .mlCalendar_management with single underscores. While I think there are BEM variants which use single underscores, I'm not seeing that in our BEM spec. So depending on the context, I think these classes need to be named either .mlCalendarList or .mlCalendar__list, depending on how they're nested.

elasticmachine · 2018-12-12T17:32:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-12-12T18:00:34Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Create calendar list in react * wip: create new_calendar page * Update new calendar settings directory name * Edit button action + update utils * Adds ability to create new calendar * Display calendar data on edit * rename directory to settings/calendar * Add scss files to calendar dir * Create new group from form * Adds event table and partial event modal. * adds datepicker to modal * Time range event functionality * add import event functionality * upate new event modal design * Add error handling to list/edit * calendarId validity check * Create/delete permission. List/form style tweak * Update calendarList to match filterList * Add missing newlines in scss files * Initial tests for calendar list * Update classnames to meet guidelines * ImportedEvents component + create utils * remove unnecessary import * rename calendars dir * include past evens in import if checkbox checked * code review updates * move components into own directories * update index.scss with dir name change * skip irrelevant tests * fix unsaved event deletion. rename scss file. * Add modal tests * Show calendarId and description as header on edit * update snapshot for refactor * update classnames to BEM guidelines * Update snapshot for classname change

alvarezmelissa87 added review enhancement New value added to drive a business result :ml Feature:Anomaly Detection ML anomaly detection v6.6.0 labels Dec 5, 2018

alvarezmelissa87 mentioned this pull request Dec 5, 2018

[ML] Calendar conversion to React #25970

Closed

30 tasks

peteharverson added the v7.0.0 label Dec 6, 2018

alvarezmelissa87 requested review from snide, peteharverson, walterra and jgowdyelastic December 6, 2018 15:54

jgowdyelastic reviewed Dec 6, 2018

View reviewed changes

x-pack/plugins/ml/public/settings/_index.scss Outdated Show resolved Hide resolved

snide reviewed Dec 6, 2018

View reviewed changes

jgowdyelastic reviewed Dec 6, 2018

View reviewed changes

peteharverson reviewed Dec 10, 2018

View reviewed changes

peteharverson reviewed Dec 11, 2018

View reviewed changes

x-pack/plugins/ml/public/settings/_index.scss Outdated Show resolved Hide resolved

x-pack/plugins/ml/public/settings/calendars/_calendar.scss Outdated Show resolved Hide resolved

x-pack/plugins/ml/public/settings/calendars/_calendar.scss Outdated Show resolved Hide resolved

alvarezmelissa87 added 17 commits December 12, 2018 11:25

Add missing newlines in scss files

85d0678

Initial tests for calendar list

d9e0530

Update classnames to meet guidelines

38ce90c

ImportedEvents component + create utils

c8d6248

remove unnecessary import

b5d6c24

rename calendars dir

2fc753b

include past evens in import if checkbox checked

f8d7a3b

code review updates

9ed166c

move components into own directories

1ba8bc6

update index.scss with dir name change

3059db9

skip irrelevant tests

0677295

fix unsaved event deletion. rename scss file.

7f8ca52

Add modal tests

36ad67d

Show calendarId and description as header on edit

52ef6e5

update snapshot for refactor

7152675

update classnames to BEM guidelines

369f356

Update snapshot for classname change

3bbc4f7

alvarezmelissa87 force-pushed the ml-calendar-settings-react branch from 5acfca7 to 3bbc4f7 Compare December 12, 2018 17:25

alvarezmelissa87 merged commit 7f4ac5d into elastic:master Dec 12, 2018

alvarezmelissa87 mentioned this pull request Dec 12, 2018

[6.x] [ML] calendar eui conversion (#26741) #27054

Merged

alvarezmelissa87 deleted the ml-calendar-settings-react branch December 12, 2018 19:23

alvarezmelissa87 mentioned this pull request Dec 12, 2018

[ML] Remove old schedule_events/ directory #27095

Merged

elasticmachine mentioned this pull request Dec 13, 2018

[ML] Migrate Angular-based components to React #18374

Closed

88 tasks

alvarezmelissa87 mentioned this pull request Dec 14, 2018

[ML] Calendar form: Errors are not read by the screen reader #20186

Closed

peteharverson mentioned this pull request Jan 14, 2019

[ML] Removes unused ml-item-select directive #28669

Merged

elasticmachine mentioned this pull request Apr 11, 2019

[ML] Improve ML Accessibility #18278

Open

91 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] calendar eui conversion #26741

[ML] calendar eui conversion #26741

alvarezmelissa87 commented Dec 5, 2018 •

edited

Loading

elasticmachine commented Dec 5, 2018

elasticmachine commented Dec 6, 2018

snide left a comment

elasticmachine commented Dec 6, 2018

elasticmachine commented Dec 7, 2018

elasticmachine commented Dec 7, 2018

peteharverson Dec 10, 2018

alvarezmelissa87 Dec 11, 2018

peteharverson Dec 10, 2018

alvarezmelissa87 Dec 11, 2018

peteharverson Dec 10, 2018

alvarezmelissa87 Dec 10, 2018

elasticmachine commented Dec 10, 2018

elasticmachine commented Dec 10, 2018

elasticmachine commented Dec 10, 2018

elasticmachine commented Dec 10, 2018

alvarezmelissa87 commented Dec 10, 2018

elasticmachine commented Dec 11, 2018

peteharverson Dec 11, 2018

alvarezmelissa87 Dec 11, 2018

peteharverson left a comment

walterra commented Dec 11, 2018

elasticmachine commented Dec 12, 2018

elasticmachine commented Dec 12, 2018

[ML] calendar eui conversion #26741

[ML] calendar eui conversion #26741

Conversation

alvarezmelissa87 commented Dec 5, 2018 • edited Loading

Summary

Note:

Checklist

For maintainers

elasticmachine commented Dec 5, 2018

elasticmachine commented Dec 6, 2018

💔 Build Failed

snide left a comment

Choose a reason for hiding this comment

elasticmachine commented Dec 6, 2018

💔 Build Failed

elasticmachine commented Dec 7, 2018

💔 Build Failed

elasticmachine commented Dec 7, 2018

💔 Build Failed

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 Dec 10, 2018

💔 Build Failed

elasticmachine commented Dec 10, 2018

💔 Build Failed

elasticmachine commented Dec 10, 2018

💔 Build Failed

elasticmachine commented Dec 10, 2018

💔 Build Failed

alvarezmelissa87 commented Dec 10, 2018

elasticmachine commented Dec 11, 2018

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

walterra commented Dec 11, 2018

elasticmachine commented Dec 12, 2018

💔 Build Failed

elasticmachine commented Dec 12, 2018

💚 Build Succeeded

alvarezmelissa87 commented Dec 5, 2018 •

edited

Loading