[ML] Job Selector - ensure relative ganttBar width set on initial render #36249

alvarezmelissa87 · 2019-05-08T02:23:56Z

Summary

Addresses follow ups described in #35638

sets ganttBar width based on the flyout table's last column width - ensures it's set on first render.

ensures a minimum ganttBar range width of 1px so that something always shows up even when jobs cover a large time range.
breaks JobSelector into smaller components
Changes Job selector bar to display badges to the left of the link, changes the copy to read Edit job selection and adds an edit icon to the link

replaces fixed px in css with proper Eui variables
- We currently don't have in EUI the colors used for the ganttBar (dashed line and blue filled in area representing the range). Using eui colors it would look like the screenshot below. I'm happy to change it to this if those colors seem good. Otherwise we'll have to stick with the existing hex codes.

Final follow-up

I'll be breaking out the JobSelectorTable into smaller components as well and adding tests for those components so expect one last follow up 😄

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
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 · 2019-05-08T02:23:59Z

Pinging @elastic/ml-ui

elasticmachine · 2019-05-08T04:38:06Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

walterra · 2019-05-08T08:04:26Z

x-pack/plugins/ml/public/components/job_selector/_job_selector.scss

@@ -1,6 +1,5 @@
 .mlJobSelectorBar {
-  padding: 10px;
-  background-color: $euiColorLightShade
+  padding: $euiSizeS;


At the moment the padding looks like this:

I'd like to suggest to align the padding with the elements further down (filter bar, Top Influencers) and change this to $euiSize, with this additional change in _explorer.scss (0px instead of $euiSize for top padding):

.results-container { padding: 0px 0px $euiSize 0px; ...

jgowdyelastic · 2019-05-08T12:10:44Z

x-pack/plugins/ml/public/components/job_selector/job_selector.js

+      const tzConfig = config.get('dateFormat:tz');
+      const dateFormatTz = (tzConfig !== 'Browser') ? tzConfig : moment.tz.guess();
+      // Give the ganttBar a little more than a 3rd of the width of the flyout
+      const derivedWidth = Math.round(flyoutEl.current.flyout.offsetWidth / 3.6);


rather than using an estimate for the col width, the actual column width could be used by examining the table.

// get all the cols in the table header const tableHeaderCols = flyoutEl.current.flyout.querySelectorAll('table thead th'); // get the width of the last col const derivedWidth = tableHeaderCols[tableHeaderCols.length - 1].offsetWidth - 16;

Minus 16 because the cell has a padding of 8px
I couldn't find a numerical representation of $euiSize for this. the values in '@elastic/eui/dist/eui_theme_light.json'; are all strings.
Could use theme.euiSize.replace('px, '') but it's a bit hacky :)

walterra

Overall LGTM, just added a comment about aligning the job badges.

jgowdyelastic

LGTM

peteharverson

LGTM, but just want to check that it's by design that when you have more than 1 row of badges that the 'Edit job selection' button goes to the right, rather than say always to the immediate right of the last badge in the list:

elasticmachine · 2019-05-08T15:14:34Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alvarezmelissa87 · 2019-05-08T15:51:24Z

@peteharverson - re: when you have more than 1 row of badges that the 'Edit job selection' button goes to the right
Yes, that is by design. I've got as a flexItem the Id badges (which are themselves a flexgroup) and as the other flexItem the link to open the flyout. Both of these are wrapped in a containing flexgroup to ensure they are always side by side.

Because of the way the wrap works - you'll sometimes see this gap if badge is too long to fit and gets moved to the next row.

I'm happy to change the design if you'd prefer it always be after the last badge.

peteharverson

Happy to go with current layout of badges. LGTM!

cchaos

Thanks again for using EUI vars where possible!

cchaos · 2019-05-08T16:55:52Z

x-pack/plugins/ml/public/components/job_selector/_job_selector.scss

  border-left: 1px solid #d6d6d6;
  border-right: 1px solid #d6d6d6;
-  margin-bottom: -16px;
-  padding-top: 9px;
+  margin-bottom: -14px;


Is there any way this could be a EUI sizing variable instead? Is 14px a must?

Unfortunately, that's the only value I could make work for the ganttBar style.

alvarezmelissa87 · 2019-05-08T17:19:49Z

retest

elasticmachine · 2019-05-08T18:29:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alvarezmelissa87 · 2019-05-08T18:37:39Z

retest

elasticmachine · 2019-05-08T19:43:58Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-08T21:28:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…der (elastic#36249) * calc ganttBar width on first render; * Move all badges into separate components. Update css to use vars * check group not undefined * update job selector padding and base ganttBar width on columns

…der (#36249) (#36313) * calc ganttBar width on first render; * Move all badges into separate components. Update css to use vars * check group not undefined * update job selector padding and base ganttBar width on columns

alvarezmelissa87 added non-issue Indicates to automation that a pull request should not appear in the release notes :ml Feature:ml-results legacy - do not use v7.2.0 labels May 8, 2019

alvarezmelissa87 requested review from walterra, peteharverson and jgowdyelastic May 8, 2019 02:23

alvarezmelissa87 requested review from a team as code owners May 8, 2019 02:23

walterra reviewed May 8, 2019

View reviewed changes

jgowdyelastic reviewed May 8, 2019

View reviewed changes

walterra approved these changes May 8, 2019

View reviewed changes

jgowdyelastic approved these changes May 8, 2019

View reviewed changes

peteharverson reviewed May 8, 2019

View reviewed changes

peteharverson approved these changes May 8, 2019

View reviewed changes

cchaos approved these changes May 8, 2019

View reviewed changes

alvarezmelissa87 added 4 commits May 8, 2019 16:16

calc ganttBar width on first render;

e117ba5

Move all badges into separate components. Update css to use vars

8689b7b

check group not undefined

f8e1eef

update job selector padding and base ganttBar width on columns

43ff7d7

alvarezmelissa87 force-pushed the ml-job-selector-follow-ups branch from fe25720 to 43ff7d7 Compare May 8, 2019 20:16

alvarezmelissa87 merged commit d4540b2 into elastic:master May 8, 2019

alvarezmelissa87 mentioned this pull request May 8, 2019

[7.x] [ML] Job Selector - ensure relative ganttBar width set on initial render (#36249) #36313

Merged

alvarezmelissa87 deleted the ml-job-selector-follow-ups branch May 8, 2019 21:48

sophiec20 added the Feature:Anomaly Detection ML anomaly detection label Jun 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Job Selector - ensure relative ganttBar width set on initial render #36249

[ML] Job Selector - ensure relative ganttBar width set on initial render #36249

alvarezmelissa87 commented May 8, 2019 •

edited

Loading

elasticmachine commented May 8, 2019

elasticmachine commented May 8, 2019

walterra May 8, 2019

jgowdyelastic May 8, 2019 •

edited

Loading

walterra left a comment

jgowdyelastic left a comment

peteharverson left a comment

elasticmachine commented May 8, 2019

alvarezmelissa87 commented May 8, 2019

peteharverson left a comment

cchaos left a comment

cchaos May 8, 2019

alvarezmelissa87 May 8, 2019

alvarezmelissa87 commented May 8, 2019

elasticmachine commented May 8, 2019

alvarezmelissa87 commented May 8, 2019

elasticmachine commented May 8, 2019

elasticmachine commented May 8, 2019

[ML] Job Selector - ensure relative ganttBar width set on initial render #36249

[ML] Job Selector - ensure relative ganttBar width set on initial render #36249

Conversation

alvarezmelissa87 commented May 8, 2019 • edited Loading

Summary

Final follow-up

Checklist

For maintainers

elasticmachine commented May 8, 2019

elasticmachine commented May 8, 2019

💚 Build Succeeded

walterra May 8, 2019

Choose a reason for hiding this comment

jgowdyelastic May 8, 2019 • edited Loading

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

elasticmachine commented May 8, 2019

💔 Build Failed

alvarezmelissa87 commented May 8, 2019

peteharverson left a comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

cchaos May 8, 2019

Choose a reason for hiding this comment

alvarezmelissa87 May 8, 2019

Choose a reason for hiding this comment

alvarezmelissa87 commented May 8, 2019

elasticmachine commented May 8, 2019

💔 Build Failed

alvarezmelissa87 commented May 8, 2019

elasticmachine commented May 8, 2019

💔 Build Failed

elasticmachine commented May 8, 2019

💚 Build Succeeded

alvarezmelissa87 commented May 8, 2019 •

edited

Loading

jgowdyelastic May 8, 2019 •

edited

Loading