Skip to content

Commit

Permalink
Unify criteria for externa/internal references for ReferenceLink and …
Browse files Browse the repository at this point in the history
…ReferencesButton components

Signed-off-by: Ruben Vargas <[email protected]>
  • Loading branch information
rubenvp8510 committed Dec 12, 2019
1 parent c4d7b5b commit 24a3759
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ describe(ReferencesButton, () => {
);

const baseProps = {
traceID: trace.traceID,
trace: {
data: trace,
},
focusSpan: () => {},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import ReferenceLink from '../url/ReferenceLink';

type TReferencesButtonProps = {
references: SpanReference[];
traceID: string;
children: React.ReactNode;
tooltipText: string;
focusSpan: (spanID: string) => void;
Expand All @@ -38,7 +37,6 @@ export default class ReferencesButton extends React.PureComponent<TReferencesBut
<Menu.Item key={`${spanID}`}>
<ReferenceLink
reference={ref}
traceID={this.props.traceID}
focusSpan={this.props.focusSpan}
className="ReferencesButton--TraceRefLink"
>
Expand Down Expand Up @@ -76,12 +74,7 @@ export default class ReferencesButton extends React.PureComponent<TReferencesBut
const ref = references[0];
return (
<Tooltip {...tooltipProps}>
<ReferenceLink
reference={ref}
traceID={this.props.traceID}
focusSpan={focusSpan}
className="ReferencesButton-MultiParent"
>
<ReferenceLink reference={ref} focusSpan={focusSpan} className="ReferencesButton-MultiParent">
{children}
</ReferenceLink>
</Tooltip>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ export default class SpanBarRow extends React.PureComponent<SpanBarRowProps> {
{span.references && span.references.length > 1 && (
<ReferencesButton
references={span.references}
traceID={span.traceID}
tooltipText="Contains multiple references"
focusSpan={focusSpan}
>
Expand All @@ -167,7 +166,6 @@ export default class SpanBarRow extends React.PureComponent<SpanBarRowProps> {
{span.subsidiarilyReferencedBy && span.subsidiarilyReferencedBy.length > 0 && (
<ReferencesButton
references={span.subsidiarilyReferencedBy}
traceID={span.traceID}
tooltipText={`This span is referenced by ${
span.subsidiarilyReferencedBy.length === 1 ? 'another span' : 'multiple other spans'
}`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ describe('<References>', () => {

const props = {
data: references,
traceID: 'trace1',
focusSpan: jest.fn(),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,25 @@ type AccordianReferencesProps = {
interactive?: boolean;
isOpen: boolean;
onToggle?: null | (() => void);
traceID: string;
focusSpan: (uiFind: string) => void;
};

type ReferenceItemProps = {
data: SpanReference[];
focusSpan: (uiFind: string) => void;
traceID: string;
};

// export for test
export function References(props: ReferenceItemProps) {
const { data, focusSpan, traceID } = props;
const { data, focusSpan } = props;

return (
<div className="ReferencesList u-simple-scrollbars">
<ul className="ReferencesList--List">
{data.map(reference => {
return (
<li className="ReferencesList--Item" key={`${reference.spanID}`}>
<ReferenceLink reference={reference} traceID={traceID} focusSpan={focusSpan}>
<ReferenceLink reference={reference} focusSpan={focusSpan}>
<span className="ReferencesList--itemContent">
{reference.span ? (
<span>
Expand Down Expand Up @@ -81,7 +80,7 @@ export default class AccordianReferences extends React.PureComponent<AccordianRe
};

render() {
const { data, highContrast, interactive, isOpen, onToggle, focusSpan, traceID } = this.props;
const { data, highContrast, interactive, isOpen, onToggle, focusSpan } = this.props;
const isEmpty = !Array.isArray(data) || !data.length;
const iconCls = cx('u-align-icon', { 'AccordianKReferences--emptyIcon': isEmpty });
let arrow: React.ReactNode | null = null;
Expand Down Expand Up @@ -110,7 +109,7 @@ export default class AccordianReferences extends React.PureComponent<AccordianRe
</strong>{' '}
({data.length})
</div>
{isOpen && <References data={data} focusSpan={focusSpan} traceID={traceID} />}
{isOpen && <References data={data} focusSpan={focusSpan} />}
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ export default function SpanDetail(props: SpanDetailProps) {
data={references}
isOpen={isReferencesOpen}
onToggle={() => referencesToggle(spanID)}
traceID={span.traceID}
focusSpan={focusSpan}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ import ReferenceLink from './ReferenceLink';

describe(ReferenceLink, () => {
const focusMock = jest.fn();
const traceID = 'trace1';

const sameTraceRef = {
refType: 'CHILD_OF',
traceID: 'trace1',
spanID: 'span1',
span: {
// not null or undefined is an indicator of an internal reference
},
};

const externalRef = {
Expand All @@ -35,28 +37,22 @@ describe(ReferenceLink, () => {

describe('rendering', () => {
it('render for this trace', () => {
const component = shallow(
<ReferenceLink reference={sameTraceRef} traceID={traceID} focusSpan={focusMock} />
);
const component = shallow(<ReferenceLink reference={sameTraceRef} focusSpan={focusMock} />);
const link = component.find('a');
expect(link.length).toBe(1);
expect(link.props().role).toBe('button');
});

it('render for external trace', () => {
const component = shallow(
<ReferenceLink reference={externalRef} traceID={traceID} focusSpan={focusMock} />
);
const component = shallow(<ReferenceLink reference={externalRef} focusSpan={focusMock} />);
const link = component.find('a[href="/trace/trace2/uiFind?=span2"]');
expect(link.length).toBe(1);
});
});
describe('focus span', () => {
it('call focusSpan', () => {
focusMock.mockReset();
const component = shallow(
<ReferenceLink reference={sameTraceRef} traceID={traceID} focusSpan={focusMock} />
);
const component = shallow(<ReferenceLink reference={sameTraceRef} focusSpan={focusMock} />);
const link = component.find('a');
link.simulate('click');
expect(focusMock).toHaveBeenLastCalledWith('span1');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { getUrl } from '.';

type ReferenceLinkProps = {
reference: SpanReference;
traceID: string;
children: React.ReactNode;
className?: string;
focusSpan: (spanID: string) => void;
Expand All @@ -28,9 +27,9 @@ type ReferenceLinkProps = {
const linkToExternalSpan = (traceID: string, spanID: string) => `${getUrl(traceID)}/uiFind?=${spanID}`;

export default function ReferenceLink(props: ReferenceLinkProps) {
const { traceID, reference, children, className, focusSpan, ...otherProps } = props;
const { reference, children, className, focusSpan, ...otherProps } = props;
delete otherProps.onClick;
if (traceID === reference.traceID) {
if (reference.span) {
return (
<a role="button" onClick={() => focusSpan(reference.spanID)} className={className} {...otherProps}>
{children}
Expand Down

0 comments on commit 24a3759

Please sign in to comment.