Skip to content

Commit

Permalink
[ML] Fixes Anomaly Explorer IE11 issues (#23558)
Browse files Browse the repository at this point in the history
Fixes two issues in IE11 for Anomaly Explorer:
- The format of the string returned from element.attr('transform') is different in IE11 so the regex based on it would fail. This fixes the issue and adds tests for the different formats. The code was also changed to gracefully return NaN in case the regex wouldn't return results, the previous version triggered a JS error.
- The migration of the swimlanes to React caused the cell selection to malfunction in IE11. This fixes it by updating the dragSelect library to use the new method setSelectables. The previous method we used (addSelectables) didn't play well with how React rerenders the swimlanes. Note this lib update using the new method will require to run yarn kbn bootstrap.
  • Loading branch information
walterra authored Sep 27, 2018
1 parent ecaf26e commit 7e4e0cb
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 18 deletions.
2 changes: 1 addition & 1 deletion x-pack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@
"d3": "3.5.6",
"d3-scale": "1.0.6",
"dedent": "^0.7.0",
"dragselect": "1.7.17",
"dragselect": "1.8.1",
"elasticsearch": "^15.1.1",
"extract-zip": "1.5.0",
"file-saver": "^1.3.8",
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/ml/public/explorer/explorer_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ module.controller('MlExplorerController', function (
$scope.queryFilters = [];

const dragSelect = new DragSelect({
selectables: document.querySelectorAll('.sl-cell'),
selectables: document.getElementsByClassName('sl-cell'),
callback(elements) {
if (elements.length > 1 && !ALLOW_CELL_RANGE_SELECTION) {
elements = [elements[0]];
Expand Down Expand Up @@ -508,7 +508,8 @@ module.controller('MlExplorerController', function (

// Listens to render updates of the swimlanes to update dragSelect
const swimlaneRenderDoneListener = function () {
dragSelect.addSelectables(document.querySelectorAll('.sl-cell'));
dragSelect.clearSelection();
dragSelect.setSelectables(document.getElementsByClassName('sl-cell'));
};
mlExplorerDashboardService.swimlaneRenderDone.watch(swimlaneRenderDoneListener);

Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/ml/public/explorer/explorer_swimlane.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,6 @@ export class ExplorerSwimlane extends React.Component {
}

clearSelection() {
const { mlExplorerDashboardService } = this.props;

// This selects both overall and viewby swimlane
const wrapper = d3.selectAll('.ml-explorer-swimlane');

Expand All @@ -212,8 +210,6 @@ export class ExplorerSwimlane extends React.Component {
wrapper.selectAll('.sl-cell-inner.sl-cell-inner-selected').classed('sl-cell-inner-selected', false);
wrapper.selectAll('.sl-cell-inner-dragselect.sl-cell-inner-selected').classed('sl-cell-inner-selected', false);
wrapper.selectAll('.ds-selected').classed('sl-cell-inner-selected', false);

mlExplorerDashboardService.swimlaneCellClick.changed({});
}

renderSwimlane() {
Expand Down
20 changes: 15 additions & 5 deletions x-pack/plugins/ml/public/util/chart_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,20 @@ export function getTickValues(startTimeMs, tickInterval, earliest, latest) {
return tickValues;
}

// To get xTransform it would be nicer to use d3.transform, but that doesn't play well with JSDOM.
// So this uses a regex variant because we definitely want test coverage for the label removal.
// Once JSDOM supports SVGAnimatedTransformList we can use this simpler inline version:
// const xTransform = d3.transform(tick.attr('transform')).translate[0];
export function getXTransform(t) {
const regexResult = /translate\(\s*([^\s,)]+)([ ,]([^\s,)]+))?\)/.exec(t);
if (Array.isArray(regexResult) && regexResult.length >= 2) {
return Number(regexResult[1]);
}

// fall back to NaN if regex didn't return any results.
return NaN;
}

// This removes overlapping x-axis labels by starting off from a specific label
// that is required/wanted to show up. The code then traverses to both sides along the axis
// and decides which labels to keep or remove. All vertical tick lines will be kept visible,
Expand Down Expand Up @@ -266,11 +280,7 @@ export function removeLabelOverlap(axis, startTimeMs, tickInterval, width) {

const tickWidth = textNode.getBBox().width;
const padding = 15;
// To get xTransform it would be nicer to use d3.transform, but that doesn't play well with JSDOM.
// So this uses a regex variant because we definitely want test coverage for the label removal.
// Once JSDOM supports SVGAnimatedTransformList we can use the simpler version.
// const xTransform = d3.transform(tick.attr('transform')).translate[0];
const xTransform = +(/translate\(\s*([^\s,)]+)[ ,]([^\s,)]+)\)/.exec(tick.attr('transform'))[1]);
const xTransform = getXTransform(tick.attr('transform'));
const xMinOffset = xTransform - (tickWidth / 2 + padding);
const xMaxOffset = xTransform + (tickWidth / 2 + padding);

Expand Down
23 changes: 23 additions & 0 deletions x-pack/plugins/ml/public/util/chart_utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { timefilter } from 'ui/timefilter';
import {
getExploreSeriesLink,
getTickValues,
getXTransform,
removeLabelOverlap
} from './chart_utils';

Expand Down Expand Up @@ -133,6 +134,28 @@ describe('getTickValues', () => {
});
});

describe('getXTransform', () => {
const expectedXTransform = 0.007167499999999999;

test('Chrome/Safari/Firefox String variant.', () => {
const transformStr = 'translate(0.007167499999999999,0)';
const xTransform = getXTransform(transformStr);
expect(xTransform).toEqual(expectedXTransform);
});

test('IE11 String variant.', () => {
const transformStr = 'translate(0.007167499999999999)';
const xTransform = getXTransform(transformStr);
expect(xTransform).toEqual(expectedXTransform);
});

test('Invalid String.', () => {
const transformStr = 'translate()';
const xTransform = getXTransform(transformStr);
expect(xTransform).toEqual(NaN);
});
});

describe('removeLabelOverlap', () => {
const originalGetBBox = SVGElement.prototype.getBBox;

Expand Down
6 changes: 3 additions & 3 deletions x-pack/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2345,9 +2345,9 @@ [email protected]:
version "2.0.0"
resolved "https://registry.yarnpkg.com/dotenv/-/dotenv-2.0.0.tgz#bd759c357aaa70365e01c96b7b0bec08a6e0d949"

dragselect@1.7.17:
version "1.7.17"
resolved "https://registry.yarnpkg.com/dragselect/-/dragselect-1.7.17.tgz#ab98661d8599286c0ada66ce5f5923b06b4f09fd"
dragselect@1.8.1:
version "1.8.1"
resolved "https://registry.yarnpkg.com/dragselect/-/dragselect-1.8.1.tgz#63f71a6f980f710c87e28b328e175b7afc9e162b"

[email protected], duplexer2@~0.0.2:
version "0.0.2"
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4223,9 +4223,9 @@ download@^5.0.3:
mkdirp "^0.5.1"
pify "^2.3.0"

dragselect@1.7.17:
version "1.7.17"
resolved "https://registry.yarnpkg.com/dragselect/-/dragselect-1.7.17.tgz#ab98661d8599286c0ada66ce5f5923b06b4f09fd"
dragselect@1.8.1:
version "1.8.1"
resolved "https://registry.yarnpkg.com/dragselect/-/dragselect-1.8.1.tgz#63f71a6f980f710c87e28b328e175b7afc9e162b"

[email protected]:
version "3.7.0"
Expand Down

0 comments on commit 7e4e0cb

Please sign in to comment.