Skip to content
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
32db185
[wip] show status code/text in editor output for multiple requests
May 19, 2022
df0b1e2
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine May 19, 2022
a63deb5
Render status badges in the output panel
May 20, 2022
0bf0c1a
Add missing dependancy for useEffect hook
May 20, 2022
215eb44
Lint
May 20, 2022
ad63eaf
Address comments
May 24, 2022
ed198f8
Add functional tests
May 24, 2022
962da8f
Update tests
May 24, 2022
5a0d907
Fix tests
May 24, 2022
574bf1d
Update selectAll command key for firefox
May 24, 2022
f56ec79
Merge branch 'main' into console/multiple-requests-ux
kibanamachine May 24, 2022
da63f46
Fix selecting all requests in tests
May 25, 2022
d352a85
Merge branch 'main' into console/multiple-requests-ux
kibanamachine May 25, 2022
0e72f31
Convert output modes to TS
May 25, 2022
15d683e
Fix checks
May 25, 2022
3aa0043
Address comments
May 26, 2022
c7b4f64
Merge branch 'main' into console/multiple-requests-ux
kibanamachine May 26, 2022
eaaee4b
Fix types for editor session.update method
May 26, 2022
371ce42
Add tests for mapStatusCodeToBadge function
May 27, 2022
18e0e7e
Merge branch 'main' into console/multiple-requests-ux
kibanamachine May 30, 2022
30cce66
Merge branch 'main' into console/multiple-requests-ux
kibanamachine Jun 1, 2022
ad57319
Address comments
Jun 3, 2022
2f9b9bc
Merge branch 'main' into console/multiple-requests-ux
kibanamachine Jun 3, 2022
0f33fd6
Merge branch 'main' into console/multiple-requests-ux
kibanamachine Jun 6, 2022
4996043
Change .ace-badge font family to $euiFontFamily
Jun 6, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { EuiScreenReaderOnly } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React, { useEffect, useRef } from 'react';
import { convertMapboxVectorTileToJson } from './mapbox_vector_tile';
import { Mode } from '../../../../models/legacy_core_editor/mode/output';

// Ensure the modes we might switch to dynamically are available
import 'brace/mode/text';
Expand Down Expand Up @@ -83,7 +84,10 @@ function EditorOutputUI() {
useEffect(() => {
const editor = editorInstanceRef.current!;
if (data) {
const mode = modeForContentType(data[0].response.contentType);
const isMultipleRequest = data.length > 1;
const mode = isMultipleRequest
? new Mode()
: modeForContentType(data[0].response.contentType);
editor.update(
data
.map((result) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
import type { HttpSetup, IHttpFetchError } from '@kbn/core/public';
import { XJson } from '@kbn/es-ui-shared-plugin/public';
import { extractWarningMessages } from '../../../lib/utils';
// @ts-ignore
import * as es from '../../../lib/es/es';
import { send } from '../../../lib/es/es';
import { BaseResponseType } from '../../../types';

const { collapseLiteralStrings } = XJson;
Expand Down Expand Up @@ -72,7 +71,7 @@ export function sendRequest(args: RequestArgs): Promise<RequestResult[]> {
const startTime = Date.now();

try {
const { response, body } = await es.send({
const { response, body } = await send({
http: args.http,
method,
path,
Expand Down Expand Up @@ -106,7 +105,7 @@ export function sendRequest(args: RequestArgs): Promise<RequestResult[]> {
}

if (isMultiRequest) {
value = '# ' + req.method + ' ' + req.url + '\n' + value;
value = `# ${req.method} ${req.url} ${response.status} ${response.statusText}\n${value}`;
}

results.push({
Expand Down Expand Up @@ -141,7 +140,7 @@ export function sendRequest(args: RequestArgs): Promise<RequestResult[]> {
}

if (isMultiRequest) {
value = '# ' + req.method + ' ' + req.url + '\n' + value;
value = `# ${req.method} ${req.url} ${statusCode} ${statusText}\n${value}`;
}

const result = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@

import _ from 'lodash';
import ace from 'brace';
// @ts-ignore
import * as OutputMode from './mode/output';
import { Mode } from './mode/output';
import smartResize from './smart_resize';

export interface CustomAceEditor extends ace.Editor {
update: (text: string, mode?: unknown, cb?: () => void) => void;
update: (text: string, mode?: string | Mode, cb?: () => void) => void;
append: (text: string, foldPrevious?: boolean, cb?: () => void) => void;
}

Expand All @@ -24,19 +23,23 @@ export interface CustomAceEditor extends ace.Editor {
export function createReadOnlyAceEditor(element: HTMLElement): CustomAceEditor {
const output: CustomAceEditor = ace.acequire('ace/ace').edit(element);

const outputMode = new OutputMode.Mode();
const outputMode = new Mode();

output.$blockScrolling = Infinity;
output.resize = smartResize(output);
output.update = (val: string, mode?: unknown, cb?: () => void) => {
output.update = (val, mode, cb) => {
if (typeof mode === 'function') {
cb = mode as () => void;
mode = void 0;
}

const session = output.getSession();
const currentMode = val ? mode || outputMode : 'ace/mode/text';

session.setMode(val ? mode || outputMode : 'ace/mode/text');
// @ts-ignore
// ignore ts error here due to type definition mistake in brace for setMode(mode: string): void;
// this method accepts string or SyntaxMode which is an object. See https://github.com/ajaxorg/ace/blob/13dc911dbc0ea31ca343d5744b3f472767458fc3/ace.d.ts#L467
session.setMode(currentMode);
session.setValue(val);
if (typeof cb === 'function') {
setTimeout(cb);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,24 @@ import ace from 'brace';

import { OutputJsonHighlightRules } from './output_highlight_rules';

const oop = ace.acequire('ace/lib/oop');
const JSONMode = ace.acequire('ace/mode/json').Mode;
const MatchingBraceOutdent = ace.acequire('ace/mode/matching_brace_outdent').MatchingBraceOutdent;
const CstyleBehaviour = ace.acequire('ace/mode/behaviour/cstyle').CstyleBehaviour;
const CStyleFoldMode = ace.acequire('ace/mode/folding/cstyle').FoldMode;
ace.acequire('ace/worker/worker_client');
const AceTokenizer = ace.acequire('ace/tokenizer').Tokenizer;

export function Mode() {
this.$tokenizer = new AceTokenizer(new OutputJsonHighlightRules().getRules());
this.$outdent = new MatchingBraceOutdent();
this.$behaviour = new CstyleBehaviour();
this.foldingRules = new CStyleFoldMode();
export class Mode extends JSONMode {
constructor() {
super();
this.$tokenizer = new AceTokenizer(new OutputJsonHighlightRules().getRules());
this.$outdent = new MatchingBraceOutdent();
this.$behaviour = new CstyleBehaviour();
this.foldingRules = new CStyleFoldMode();
}
}
oop.inherits(Mode, JSONMode);

(function () {
(function (this: Mode) {
this.createWorker = function () {
return null;
};
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { mapStatusCodeToBadge } from './output_highlight_rules';

describe('mapStatusCodeToBadge', () => {
const testCases = [
{
description: 'treats 100 as as default',
value: '# PUT test-index 100 Continue',
badge: 'badge.badge--default',
},
{
description: 'treats 200 as success',
value: '# PUT test-index 200 OK',
badge: 'badge.badge--success',
},
{
description: 'treats 301 as primary',
value: '# PUT test-index 301 Moved Permanently',
badge: 'badge.badge--primary',
},
{
description: 'treats 400 as warning',
value: '# PUT test-index 404 Not Found',
badge: 'badge.badge--warning',
},
{
description: 'treats 502 as danger',
value: '# PUT test-index 502 Bad Gateway',
badge: 'badge.badge--danger',
},
{
description: 'treats unexpected numbers as danger',
value: '# PUT test-index 666 Demonic Invasion',
badge: 'badge.badge--danger',
},
{
description: 'treats no numbers as undefined',
value: '# PUT test-index',
badge: undefined,
},
];

testCases.forEach(({ description, value, badge }) => {
test(description, () => {
expect(mapStatusCodeToBadge(value)).toBe(badge);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import ace from 'brace';
import 'brace/mode/json';
import { addXJsonToRules } from '@kbn/ace';

const JsonHighlightRules = ace.acequire('ace/mode/json_highlight_rules').JsonHighlightRules;

export const mapStatusCodeToBadge = (value: string) => {
const regExpMatchArray = value.match(/\d+/);
if (regExpMatchArray) {
const status = parseInt(regExpMatchArray[0], 10);
if (status <= 199) {
return 'badge.badge--default';
}
if (status <= 299) {
return 'badge.badge--success';
}
if (status <= 399) {
return 'badge.badge--primary';
}
if (status <= 499) {
return 'badge.badge--warning';
}
return 'badge.badge--danger';
}
};

export class OutputJsonHighlightRules extends JsonHighlightRules {
constructor() {
super();
this.$rules = {};
addXJsonToRules(this, 'start');
this.$rules.start.unshift(
{
token: 'warning',
regex: '#!.*$',
},
{
token: 'comment',
regex: /#(.*?)(?=\d+\s(?:[\sA-Za-z]+)|$)/,
},
{
token: mapStatusCodeToBadge,
regex: /(\d+\s[\sA-Za-z]+$)/,
}
);

if (this instanceof OutputJsonHighlightRules) {
this.normalizeRules();
}
}
}
41 changes: 41 additions & 0 deletions src/plugins/console/public/styles/_app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,47 @@
.conApp__output {
display: flex;
flex: 1 1 1px;

.ace_badge {
font-size: 12px;
font-weight: 500;
line-height: 18px;
padding: 0 8px;
display: inline-block;
text-decoration: none;
border-radius: 3px;
border: solid 1px transparent;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this style? If not serving one, can potentially be removed.

white-space: nowrap;
vertical-align: middle;
cursor: default;
max-width: 100%;
text-align: left;
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this style can be removed, as its left text alignment is already being inherited and applied.

Suggested change
text-align: left;


&--success {
background-color: $euiColorSuccess;
color: chooseLightOrDarkText($euiColorSuccess);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggesting change to the correct badge background colors here.

Suggested change
background-color: $euiColorSuccess;
color: chooseLightOrDarkText($euiColorSuccess);
background-color: $euiColorVis0_behindText;
color: chooseLightOrDarkText($euiColorVis0_behindText);

Copy link
Contributor

Choose a reason for hiding this comment

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

@MichaelMarcialis For our own edification, can you teach us about the role of $euiColorVis0_behindText and similar colors, and what makes it better than $euiColorSuccess here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question, @cjcenizal! For the purposes of this PR, my reason for the suggested color changes were purely to make these ace badges consistent with the colors that our EuiBadge components use.

That said, I imagine the reason the EUI team ultimately chose to use the behind-text visualization colors for the EuiBadge component has to do with accessibility and the contrast ratio, as this component houses fairly small sized text. The contrast with the behind-text visualization colors appears to be higher and receives a better WCAG rating. Unlike EuiBadge, I assume that components with larger text sizes (such as EuiButton) can afford be a bit less strict with the color contrast, which is why they can use colors like $euiColorSuccess.

CCing @cchaos here to keep me honest and double-check my assumptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

For full context on why we used visualization colors for badges, you can read through this PR: elastic/eui#2455

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you both! This helps me out a ton.

}

&--warning {
background-color: $euiColorWarning;
color: chooseLightOrDarkText($euiColorWarning);
}

&--primary {
background-color: $euiColorPrimary;
color: chooseLightOrDarkText($euiColorPrimary);
}

&--default {
background-color: $euiColorGhost;
color: chooseLightOrDarkText($euiColorGhost);
}

&--danger {
background-color: $euiColorDanger;
color: chooseLightOrDarkText($euiColorDanger);
}
}
}

.conApp__editorContent,
Expand Down
34 changes: 33 additions & 1 deletion test/functional/apps/console/_console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import expect from '@kbn/expect';
import { asyncForEach } from '@kbn/std';
import { FtrProviderContext } from '../../ftr_provider_context';

const DEFAULT_REQUEST = `
Expand All @@ -24,7 +25,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const retry = getService('retry');
const log = getService('log');
const browser = getService('browser');
const PageObjects = getPageObjects(['common', 'console']);
const PageObjects = getPageObjects(['common', 'console', 'header']);
const toasts = getService('toasts');

describe('console app', function describeIndexTests() {
Expand Down Expand Up @@ -122,5 +123,36 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
});
});

describe('multiple requests output', () => {
const sendMultipleRequests = async (requests: string[]) => {
await asyncForEach(requests, async (request) => {
await PageObjects.console.enterRequest(request);
});
await PageObjects.console.selectAllRequests();
await PageObjects.console.clickPlay();
};

beforeEach(async () => {
await PageObjects.console.clearTextArea();
});

it('should contain comments starting with # symbol', async () => {
await sendMultipleRequests(['\n PUT test-index', '\n DELETE test-index']);
await retry.try(async () => {
const response = await PageObjects.console.getResponse();
log.debug(response);
expect(response).to.contain('# PUT test-index 200 OK');
expect(response).to.contain('# DELETE test-index 200 OK');
});
});

it('should display status badges', async () => {
await sendMultipleRequests(['\n GET _search/test', '\n GET _search']);
await PageObjects.header.waitUntilLoadingHasFinished();
expect(await PageObjects.console.hasWarningBadge()).to.be(true);
expect(await PageObjects.console.hasSuccessBadge()).to.be(true);
});
});
});
}
Loading