Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding sass lint to Monitoring #44148

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
eee283c
basic cleanup
andreadelrio Aug 21, 2019
25923f0
will try to use EuiIcons for status
andreadelrio Aug 21, 2019
cb70628
replaced fa-circle with EuiIcon dot
andreadelrio Aug 23, 2019
7ebd6b6
made flexitem and flexgroup available
andreadelrio Aug 27, 2019
bd8e8c7
chart legend overlaps
andreadelrio Aug 27, 2019
1a2c015
Merge remote-tracking branch 'upstream/master' into sass-lint-monitoring
andreadelrio Aug 28, 2019
f67df41
some changes
andreadelrio Sep 6, 2019
03cb97f
Merge remote-tracking branch 'upstream/master' into sass-lint-monitoring
andreadelrio Sep 8, 2019
d0c1a36
no sass lint warnings
andreadelrio Sep 9, 2019
c0da56d
Merge remote-tracking branch 'upstream/master' into sass-lint-monitoring
andreadelrio Sep 10, 2019
b81d49e
Merge remote-tracking branch 'upstream/master' into sass-lint-monitoring
andreadelrio Sep 10, 2019
0089c0d
[SIEM] Changes ML conditional links to use tabs, fixes a small bug wi…
FrankHassanabad Sep 10, 2019
b58e43b
replaced number with eui variable
andreadelrio Sep 10, 2019
3c00e62
Merge remote-tracking branch 'upstream/master' into sass-lint-monitoring
andreadelrio Sep 12, 2019
93c0966
Merge remote-tracking branch 'upstream/master' into sass-lint-monitoring
andreadelrio Sep 12, 2019
58d717c
using euiBorderColor for charts
andreadelrio Sep 17, 2019
dcaeb4d
Merge remote-tracking branch 'upstream/master' into sass-lint-monitoring
andreadelrio Sep 17, 2019
62b3d54
fix lint error
andreadelrio Sep 17, 2019
b66c3d7
Fix tests
chrisronline Sep 17, 2019
694d925
Fix more tests
chrisronline Sep 18, 2019
1196745
Merge remote-tracking branch 'elastic/master' into sass-lint-monitoring
chrisronline Sep 18, 2019
1fc9186
small sass changes
andreadelrio Sep 25, 2019
eb6fbf2
trying new layout
andreadelrio Sep 25, 2019
4835224
new layout for shard legend
andreadelrio Sep 27, 2019
0b4bc8b
Merge branch 'sass-lint-monitoring' of https://github.com/andreadelri…
andreadelrio Sep 27, 2019
950afee
flexgrid for unassigned
andreadelrio Sep 27, 2019
fb1e478
fix conflict
andreadelrio Sep 27, 2019
47e294c
snapshots
andreadelrio Sep 27, 2019
40aea9a
refining layout
andreadelrio Oct 2, 2019
4131d13
fixed conflict
andreadelrio Oct 3, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .sass-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ files:
- 'x-pack/legacy/plugins/rollup/**/*.s+(a|c)ss'
- 'x-pack/legacy/plugins/security/**/*.s+(a|c)ss'
- 'x-pack/legacy/plugins/canvas/**/*.s+(a|c)ss'
- 'x-pack/legacy/plugins/monitoring/**/*.s+(a|c)ss'
rules:
quotes:
- 2
Expand Down
25 changes: 12 additions & 13 deletions x-pack/legacy/plugins/monitoring/common/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ export const SORT_DESCENDING = -1;
* Chart colors
* @type {string}
*/
export const CHART_LINE_COLOR = '#d2d2d2';
export const CHART_TEXT_COLOR = '#9c9c9c';

/*
Expand Down Expand Up @@ -112,7 +111,7 @@ export const CLOUD_METADATA_SERVICES = {

// GCP documentation shows both 'metadata.google.internal' (mostly) and '169.254.169.254' (sometimes)
// To bypass potential DNS changes, the IP was used because it's shared with other cloud services
GCP_URL_PREFIX: 'http://169.254.169.254/computeMetadata/v1/instance'
GCP_URL_PREFIX: 'http://169.254.169.254/computeMetadata/v1/instance',
};

/**
Expand All @@ -122,13 +121,13 @@ export const LOGSTASH = {
MAJOR_VER_REQD_FOR_PIPELINES: 6,

/*
* Names ES keys on for different Logstash pipeline queues.
* @type {string}
*/
* Names ES keys on for different Logstash pipeline queues.
* @type {string}
*/
QUEUE_TYPES: {
MEMORY: 'memory',
PERSISTED: 'persisted'
}
PERSISTED: 'persisted',
},
};

export const DEBOUNCE_SLOW_MS = 17; // roughly how long it takes to render a frame at 60fps
Expand Down Expand Up @@ -162,12 +161,12 @@ export const APM_CUSTOM_ID = 'apm';
export const INFRA_SOURCE_ID = 'internal-stack-monitoring';

/*
* These constants represent code paths within `getClustersFromRequest`
* that an api call wants to invoke. This is meant as an optimization to
* avoid unnecessary ES queries (looking at you logstash) when the data
* is not used. In the long term, it'd be nice to have separate api calls
* instead of this path logic.
*/
* These constants represent code paths within `getClustersFromRequest`
* that an api call wants to invoke. This is meant as an optimization to
* avoid unnecessary ES queries (looking at you logstash) when the data
* is not used. In the long term, it'd be nice to have separate api calls
* instead of this path logic.
*/
export const CODE_PATH_ALL = 'all';
export const CODE_PATH_ALERTS = 'alerts';
export const CODE_PATH_KIBANA = 'kibana';
Expand Down
7 changes: 1 addition & 6 deletions x-pack/legacy/plugins/monitoring/public/_hacks.scss
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
@import '@elastic/eui/src/components/header/variables';

#monitoring-app {
// SASSTODO: PUI tooltips can be replaced with EuiToolTip
.pui-tooltip-inner {
font-size: $euiFontSizeXS;
}

#monitoring-app { // sass-lint:disable-line no-ids
.monitoring-tooltip__trigger,
.monitoring-tooltip__trigger:hover {
color: $euiTextColor;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
@mixin monitoringNoUserSelect(){
@mixin monitoringNoUserSelect {
user-select: none;
-webkit-touch-callout: none;
-webkit-tap-highlight-color: transparent;
-webkit-touch-callout: none; // sass-lint:disable-line no-vendor-prefixes
-webkit-tap-highlight-color: transparent; // sass-lint:disable-line no-vendor-prefixes
}

.monRhythmChart__wrapper .monRhythmChart__zoom {
Expand All @@ -12,7 +12,7 @@
.monRhythmChart__wrapper:hover .monRhythmChart__zoom {
visibility: visible;
}

.monRhythmChart {
position: relative;
display: flex;
Expand Down Expand Up @@ -50,17 +50,20 @@

// SASSTODO: generic selector
div {
@include monitoringNoUserSelect();
@include monitoringNoUserSelect;
}
}

.monRhythmChart__legendItem {
font-size: $euiFontSizeXS;
cursor: pointer;
color: $euiTextColor;
display: flex;
flex-direction: row;
align-items: center;

&-isDisabled {
opacity: 0.5;
opacity: .5;
}
}

Expand All @@ -71,7 +74,11 @@
.monRhythmChart__legendLabel {
overflow: hidden;
white-space: nowrap;
display: flex;
flex-direction: row;
align-items: center;
}

.monRhythmChart__legendValue {
overflow: hidden;
white-space: nowrap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ export class ChartTarget extends React.Component {
}

shutdownChart() {
if (!this.plot) { return; }
if (!this.plot) {
return;
}

const { target } = this.refs;
$(target).off('plothover');
Expand All @@ -39,11 +41,11 @@ export class ChartTarget extends React.Component {

filterByShow(seriesToShow) {
if (seriesToShow) {
return (metric) => {
return metric => {
return seriesToShow.some(id => id.toLowerCase() === metric.id.toLowerCase());
};
}
return (_metric) => true;
return _metric => true;
}

componentWillReceiveProps(newProps) {
Expand Down Expand Up @@ -79,12 +81,12 @@ export class ChartTarget extends React.Component {
getOptions() {
const opts = getChartOptions({
yaxis: { tickFormatter: this.props.tickFormatter },
xaxis: this.props.timeRange
xaxis: this.props.timeRange,
});

return {
...opts,
...this.props.options
...this.props.options,
};
}

Expand All @@ -96,14 +98,16 @@ export class ChartTarget extends React.Component {
this.plot = $.plot(target, data, this.getOptions());

this._handleResize = () => {
if (!this.plot) { return; }
if (!this.plot) {
return;
}

try {
this.plot.resize();
this.plot.setupGrid();
this.plot.draw();
}
catch (e) { // eslint-disable-line no-empty
} catch (e) {
// eslint-disable-line no-empty
/* It is ok to silently swallow the error here. Resize events fire
* continuously so the proper resize will happen in a later firing of
* the event */
Expand Down Expand Up @@ -170,11 +174,9 @@ export class ChartTarget extends React.Component {
position: 'relative',
display: 'flex',
rowDirection: 'column',
flex: '1 0 auto'
flex: '1 0 auto',
};

return (
<div ref="target" style={style} />
);
return <div ref="target" style={style} />;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,53 +5,66 @@
*/

import { merge } from 'lodash';
import { CHART_LINE_COLOR, CHART_TEXT_COLOR } from '../../../common/constants';
import { CHART_TEXT_COLOR } from '../../../common/constants';
import chrome from '../../../../../../../src/legacy/ui/public/chrome';
import euiDarkVars from '@elastic/eui/dist/eui_theme_dark.json';
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
const isDarkMode = chrome.getUiSettingsClient().get('theme:darkMode');

export const euiColorForTheme = euiColor => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cchaos What's the best way to do this in Kibana these days? I suggested this based on some older code I had, and didn't know if we have anything more utility based at this point?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the correct way right now, but we should consider writing a NP plugin for this like the EUI/Charts one but specifically for the colors json files.

There are currently around 65 files right now pulling in these json files (and probably doing the dark theme check).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the latest commit of my PR, we could actually just add more methods to that since it's no longer EUI and Charts specific.

if (isDarkMode) {
return euiDarkVars[euiColor];
}
return euiLightVars[euiColor];
};

export function getChartOptions(axisOptions) {
const opts = {
legend: {
show: false
show: false,
},
xaxis: {
color: CHART_LINE_COLOR,
color: euiColorForTheme('euiBorderColor'),
timezone: 'browser',
mode: 'time', // requires `time` flot plugin
font: {
color: CHART_TEXT_COLOR
}
color: CHART_TEXT_COLOR,
},
},
yaxis: {
color: CHART_LINE_COLOR,
color: euiColorForTheme('euiBorderColor'),
font: {
color: CHART_TEXT_COLOR
}
color: CHART_TEXT_COLOR,
},
},
series: {
points: {
show: true,
radius: 1
radius: 1,
},
lines: {
show: true,
lineWidth: 2
lineWidth: 2,
},
shadowSize: 0
shadowSize: 0,
},
grid: {
margin: 0,
borderWidth: 1,
borderColor: CHART_LINE_COLOR,
hoverable: true
borderColor: euiColorForTheme('euiBorderColor'),
hoverable: true,
},
crosshair: { // requires `crosshair` flot plugin
crosshair: {
// requires `crosshair` flot plugin
mode: 'x',
color: '#c66',
lineWidth: 2
lineWidth: 2,
},
selection: { // requires `selection` flot plugin
selection: {
// requires `selection` flot plugin
mode: 'x',
color: CHART_TEXT_COLOR
}
color: CHART_TEXT_COLOR,
},
};

return merge(opts, axisOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@

import React from 'react';
import { includes, isFunction } from 'lodash';
import {
EuiKeyboardAccessible,
} from '@elastic/eui';
import { EuiFlexItem, EuiFlexGroup, EuiIcon, EuiKeyboardAccessible } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';

Expand All @@ -23,11 +21,7 @@ export class HorizontalLegend extends React.Component {
* @param {Number} value Final value to display
*/
displayValue(value) {
return (
<span className="monRhythmChart__legendValue">
{ value }
</span>
);
return <span className="monRhythmChart__legendValue">{value}</span>;
}

/**
Expand All @@ -44,10 +38,12 @@ export class HorizontalLegend extends React.Component {
*/
formatter(value, row) {
if (!this.validValue(value)) {
return (<FormattedMessage
id="xpack.monitoring.chart.horizontalLegend.notAvailableLabel"
defaultMessage="N/A"
/>);
return (
<FormattedMessage
id="xpack.monitoring.chart.horizontalLegend.notAvailableLabel"
defaultMessage="N/A"
/>
);
}

if (row && row.tickFormatter) {
Expand All @@ -61,38 +57,37 @@ export class HorizontalLegend extends React.Component {
}

createSeries(row, rowIdx) {
const classes = ['col-md-4 col-xs-6 monRhythmChart__legendItem'];
const classes = ['monRhythmChart__legendItem'];

if (!includes(this.props.seriesFilter, row.id)) {
classes.push('monRhythmChart__legendItem-isDisabled');
}
if (!row.label || row.legend === false) {
return (
<div
key={rowIdx}
style={{ display: 'none' }}
/>
);
return <div key={rowIdx} style={{ display: 'none' }} />;
}

return (
<EuiKeyboardAccessible key={rowIdx}>
<div
<EuiFlexItem
grow={false}
className={classes.join(' ')}
onClick={event => this.props.onToggle(event, row.id)}
>
<span className="monRhythmChart__legendLabel">
<span
className="fa fa-circle monRhythmChart__legendIndicator"
style={{ color: row.color }}
aria-label={i18n.translate('xpack.monitoring.chart.horizontalLegend.toggleButtonAriaLabel', {
defaultMessage: 'toggle button'
})}
<EuiIcon
className="monRhythmChart__legendIndicator"
aria-label={i18n.translate(
'xpack.monitoring.chart.horizontalLegend.toggleButtonAriaLabel',
{ defaultMessage: 'toggle button' }
)}
size="l"
type="dot"
color={row.color}
/>
{ ' ' + row.label + ' ' }
{' ' + row.label + ' '}
</span>
{ this.formatter(this.props.seriesValues[row.id], row) }
</div>
{this.formatter(this.props.seriesValues[row.id], row)}
</EuiFlexItem>
</EuiKeyboardAccessible>
);
}
Expand All @@ -102,9 +97,9 @@ export class HorizontalLegend extends React.Component {

return (
<div className="monRhythmChart__legendHorizontal">
<div className="row monRhythmChart__legend-series">
{ rows }
</div>
<EuiFlexGroup wrap={true} gutterSize="s" className="monRhythmChart__legend-series">
{rows}
</EuiFlexGroup>
</div>
);
}
Expand Down
Loading