Skip to content

Commit

Permalink
[APM] fixes elastic#23560 by separating frame headers from code previ…
Browse files Browse the repository at this point in the history
…ew and only displaying code preview when stackframe has source code context
  • Loading branch information
ogupte committed Nov 30, 2018
1 parent d46c3ab commit 5ac4119
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,20 @@ exports[`CodePreview should render with data 1`] = `
z-index: 2;
}
.c1 {
.c2 {
color: #999999;
padding: 8px;
border-bottom: 1px solid #d9d9d9;
border-radius: 5px 5px 0 0;
font-family: "SFMono-Regular",Consolas,"Liberation Mono",Menlo,Courier,monospace;
}
.c3 {
font-weight: bold;
color: #000000;
}
.c1 {
border-bottom: 1px solid #d9d9d9;
border-radius: 5px 5px 0 0;
}
.c0 {
Expand All @@ -104,36 +109,36 @@ exports[`CodePreview should render with data 1`] = `
background: #f5f5f5;
}
.c0 .c2 {
color: #000000;
}
<div
className="c0"
>
<div
className="c1"
>
<span
className="c2 c3"
>
server/coffee.js
</span>
in
<span
className="c2 c3"
>
&lt;anonymous&gt;
</span>
at
<span
className="c2 c3"
<div
className="c2"
>
line
17
</span>
<span
className="c3"
>
server/coffee.js
</span>
in
<span
className="c3"
>
&lt;anonymous&gt;
</span>
at
<span
className="c3"
>
line
17
</span>
</div>
</div>
<div
className="c4"
Expand Down
41 changes: 12 additions & 29 deletions x-pack/plugins/apm/public/components/shared/CodePreview/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,37 +22,24 @@ import python from 'react-syntax-highlighter/dist/languages/python';
import ruby from 'react-syntax-highlighter/dist/languages/ruby';
import { Variables } from './Variables';
import { Context } from './Context';
import { FrameHeading } from '../Stacktrace/FrameHeading';

registerLanguage('javascript', javascript);
registerLanguage('python', python);
registerLanguage('ruby', ruby);

const FileDetails = styled.div`
color: ${colors.gray3};
padding: ${px(units.half)};
const CodeHeader = styled.div`
border-bottom: 1px solid ${colors.gray4};
border-radius: ${borderRadius} ${borderRadius} 0 0;
`;

const FileDetail = styled.span`
font-weight: bold;
`;

const Container = styled.div`
margin: 0 0 ${px(units.plus)} 0;
position: relative;
font-family: ${fontFamilyCode};
border: 1px solid ${colors.gray4};
border-radius: ${borderRadius};
background: ${props => (props.isLibraryFrame ? colors.white : colors.gray5)};
${FileDetails} {
${props => (!props.hasContext ? 'border-bottom: 0' : null)};
}
${FileDetail} {
color: ${props => (props.isLibraryFrame ? colors.gray1 : colors.black)};
}
`;

class CodePreview extends PureComponent {
Expand All @@ -67,26 +54,22 @@ class CodePreview extends PureComponent {

render() {
const { stackframe, codeLanguage, isLibraryFrame } = this.props;
const hasContext = !(
isEmpty(stackframe.context) && isEmpty(stackframe.line.context)
);
const hasVariables = !isEmpty(stackframe.vars);

return (
<Container hasContext={hasContext} isLibraryFrame={isLibraryFrame}>
<FileDetails>
<FileDetail>{stackframe.filename}</FileDetail> in{' '}
<FileDetail>{stackframe.function}</FileDetail> at{' '}
<FileDetail>line {stackframe.line.number}</FileDetail>
</FileDetails>

{hasContext && (
<Context
<Container isLibraryFrame={isLibraryFrame}>
<CodeHeader>
<FrameHeading
stackframe={stackframe}
codeLanguage={codeLanguage}
isLibraryFrame={isLibraryFrame}
/>
)}
</CodeHeader>

<Context
stackframe={stackframe}
codeLanguage={codeLanguage}
isLibraryFrame={isLibraryFrame}
/>

{hasVariables && (
<Variables
Expand Down
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;
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import styled from 'styled-components';
import { colors, fontFamilyCode, px, units } from '../../../style/variables';

const FileDetails = styled.div`
color: ${colors.gray3};
padding: ${px(units.half)};
font-family: ${fontFamilyCode};
`;
const LibraryFrameFileDetail = styled.span`
color: ${colors.gray2};
`;
const AppFrameFileDetail = styled.span`
font-weight: bold;
color: ${colors.black};
`;

interface StackframeLine {
number: number;
}

interface Stackframe {
filename: string;
function: string;
line: StackframeLine;
}

interface Props {
stackframe: Stackframe;
isLibraryFrame?: boolean;
}

const FrameHeading: React.SFC<Props> = ({
stackframe,
isLibraryFrame = false
}) => {
const FileDetail = isLibraryFrame
? LibraryFrameFileDetail
: AppFrameFileDetail;
return (
<FileDetails>
<FileDetail>{stackframe.filename}</FileDetail> in{' '}
<FileDetail>{stackframe.function}</FileDetail> at{' '}
<FileDetail>line {stackframe.line.number}</FileDetail>
</FileDetails>
);
};

export { FrameHeading };
45 changes: 30 additions & 15 deletions x-pack/plugins/apm/public/components/shared/Stacktrace/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Ellipsis } from '../../shared/Icons';
import { units, px } from '../../../style/variables';
import { EmptyMessage } from '../../shared/EmptyMessage';
import { EuiLink, EuiTitle } from '@elastic/eui';
import { FrameHeading } from './FrameHeading';

const LibraryFrameToggle = styled.div`
margin: 0 0 ${px(units.plus)} 0;
Expand Down Expand Up @@ -39,6 +40,12 @@ function getCollapsedLibraryFrames(stackframes) {
}, []);
}

function hasSourceLines(stackframe) {
return (
!isEmpty(stackframe.context) || !isEmpty(get(stackframe, 'line.context'))
);
}

class Stacktrace extends PureComponent {
state = {
libraryframes: {}
Expand Down Expand Up @@ -79,13 +86,16 @@ class Stacktrace extends PureComponent {
</EuiTitle>
{getCollapsedLibraryFrames(stackframes).map((item, i) => {
if (!item.libraryFrame) {
return (
<CodePreview
key={i}
stackframe={item}
codeLanguage={codeLanguage}
/>
);
if (hasSourceLines(item)) {
return (
<CodePreview
key={i}
stackframe={item}
codeLanguage={codeLanguage}
/>
);
}
return <FrameHeading key={i} stackframe={item} />;
}

return (
Expand Down Expand Up @@ -115,14 +125,19 @@ function Libraryframes({ visible, stackframes, codeLanguage, onClick }) {

<LibraryFrames>
{visible &&
stackframes.map((stackframe, i) => (
<CodePreview
key={i}
stackframe={stackframe}
isLibraryFrame
codeLanguage={codeLanguage}
/>
))}
stackframes.map(
(stackframe, i) =>
hasSourceLines(stackframe) ? (
<CodePreview
key={i}
stackframe={stackframe}
isLibraryFrame
codeLanguage={codeLanguage}
/>
) : (
<FrameHeading key={i} stackframe={stackframe} isLibraryFrame />
)
)}
</LibraryFrames>
</div>
);
Expand Down

0 comments on commit 5ac4119

Please sign in to comment.