Commnets addressed

Signed-off-by: Alberto Gutierrez <aljesusg@gmail.com>

packages/jaeger-ui/src/components/App/Page.js

@@ -20,7 +20,7 @@ import Helmet from 'react-helmet';
 import { connect } from 'react-redux';
 import type { Location } from 'react-router-dom';
 import { withRouter } from 'react-router-dom';
-
+import { isEmbed } from '../../utils/embedded';
 import TopNav from './TopNav';
 import { trackPageView } from '../../utils/tracking';
 
@@ -52,17 +52,16 @@ export class PageImpl extends React.Component<Props> {
   }
 
   render() {
-    const isEmbed = this.props.search.includes('embed');
     return (
       <div>
         <Helmet title="Jaeger UI" />
         <Layout>
-          {!isEmbed && (
+          {!isEmbed(this.props.search) && (
             <Header className="Page--topNav">
               <TopNav />
             </Header>
           )}
-          <Content className={ !isEmbed && "Page--content"}>{this.props.children}</Content>
+          <Content className={!isEmbed(this.props.search) && 'Page--content'}>{this.props.children}</Content>
         </Layout>
       </div>
     );

packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.js

@@ -46,10 +46,10 @@ export default class ResultItem extends React.PureComponent<Props> {
 
   render() {
     const {
+      disableComparision,
       durationPercent,
       isInDiffCohort,
       linkTo,
-      disableComparision,
       toggleComparison,
       trace,
     } = this.props;

packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.js

@@ -15,7 +15,7 @@
 // limitations under the License.
 
 import * as React from 'react';
-import { Button, Select } from 'antd';
+import { Select, Button } from 'antd';
 import { Field, reduxForm, formValueSelector } from 'redux-form';
 
 import DiffSelection from './DiffSelection';
@@ -27,6 +27,7 @@ import * as orderBy from '../../../model/order-by';
 import { getPercentageOfDuration } from '../../../utils/date';
 import prefixUrl from '../../../utils/prefix-url';
 import reduxFormFieldAdapter from '../../../utils/redux-form-field-adapter';
+import { VERSION_API } from '../../../utils/embedded';
 
 import type { FetchedTrace } from '../../../types';
 
@@ -37,11 +38,10 @@ type SearchResultsProps = {
   cohortRemoveTrace: string => void,
   diffCohort: FetchedTrace[],
   goToTrace: string => void,
-  isEmbed?: boolean,
+  embed?: boolean,
   hideGraph?: boolean,
-  disableComparision?: boolean,
   loading: boolean,
-  onGoFullClicked: () => void,
+  getSearchURL: () => string,
   maxTraceDuration: number,
   skipMessage?: boolean,
   traces: TraceSummary[],
@@ -89,8 +89,20 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
   };
 
   render() {
-    const { loading, diffCohort, skipMessage, traces } = this.props;
-    const diffSelection = <DiffSelection toggleComparison={this.toggleComparison} traces={diffCohort} />;
+    const {
+      loading,
+      diffCohort,
+      skipMessage,
+      traces,
+      goToTrace,
+      embed,
+      hideGraph,
+      getSearchURL,
+      maxTraceDuration,
+    } = this.props;
+    const diffSelection = !embed && (
+      <DiffSelection toggleComparison={this.toggleComparison} traces={diffCohort} />
+    );
     if (loading) {
       return (
         <React.Fragment>
@@ -111,14 +123,6 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
         </React.Fragment>
       );
     }
-    const {
-      goToTrace,
-      isEmbed,
-      hideGraph,
-      disableComparision,
-      onGoFullClicked,
-      maxTraceDuration,
-    } = this.props;
     const cohortIds = new Set(diffCohort.map(datum => datum.id));
     return (
       <div>
@@ -142,9 +146,14 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
             )}
             <div className="SearchResults--headerOverview">
               <SelectSort />
-              {isEmbed && (
+              {embed && (
                 <label className="ub-right">
-                  <Button className="ub-mr2 ub-flex ub-items-center" onClick={onGoFullClicked}>
+                  <Button
+                    className="ub-mr2 ub-items-center"
+                    icon="export"
+                    target="_blank"
+                    href={getSearchURL()}
+                  >
                     View Results
                   </Button>
                 </label>
@@ -156,17 +165,23 @@ export default class SearchResults extends React.PureComponent<SearchResultsProp
           </div>
         </div>
         <div>
-          {!disableComparision && diffSelection}
+          {diffSelection}
           <ul className="ub-list-reset">
             {traces.map(trace => (
               <li className="ub-my3" key={trace.traceID}>
                 <ResultItem
                   durationPercent={getPercentageOfDuration(trace.duration, maxTraceDuration)}
                   isInDiffCohort={cohortIds.has(trace.traceID)}
-                  linkTo={prefixUrl(isEmbed ? `/trace/${trace.traceID}?embed` : `/trace/${trace.traceID}`)}
+                  linkTo={prefixUrl(
+                    embed
+                      ? `/trace/${trace.traceID}?embed=${VERSION_API}&fromSearch=${encodeURIComponent(
+                          getSearchURL()
+                        )}`
+                      : `/trace/${trace.traceID}`
+                  )}
                   toggleComparison={this.toggleComparison}
                   trace={trace}
-                  disableComparision={disableComparision}
+                  disableComparision={embed}
                 />
               </li>
             ))}

packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js

@@ -20,7 +20,6 @@ import * as markers from './index.markers';
 import ResultItem from './ResultItem';
 import ScatterPlot from './ScatterPlot';
 import LoadingIndicator from '../../common/LoadingIndicator';
-import DiffSelection from './DiffSelection';
 
 describe('<SearchResults>', () => {
   let wrapper;
@@ -54,11 +53,6 @@ describe('<SearchResults>', () => {
     expect(wrapper.find(ScatterPlot).length).toBe(0);
   });
 
-  it('hide DiffSelection if queryparam disableComparision', () => {
-    wrapper.setProps({ disableComparision: true });
-    expect(wrapper.find(DiffSelection).length).toBe(0);
-  });
-
   describe('search finished with results', () => {
     it('shows a scatter plot', () => {
       expect(wrapper.find(ScatterPlot).length).toBe(1);

packages/jaeger-ui/src/components/SearchTracePage/index.js

@@ -30,6 +30,7 @@ import { fetchedState } from '../../constants';
 import { sortTraces } from '../../model/search';
 import getLastXformCacher from '../../utils/get-last-xform-cacher';
 import prefixUrl from '../../utils/prefix-url';
+import { isEmbed } from '../../utils/embedded';
 
 import './index.css';
 import JaegerLogo from '../../img/jaeger-logo.svg';
@@ -60,15 +61,14 @@ export class SearchTracePageImpl extends Component {
   }
 
   goToTrace = traceID => {
-    const url = this.props.isEmbed ? `/trace/${traceID}?embed` : `/trace/${traceID}`;
+    const url = this.props.embed ? `/trace/${traceID}?embed` : `/trace/${traceID}`;
     this.props.history.push(prefixUrl(url));
   };
 
-  goFullView = () => {
+  getSearchURL = () => {
     const urlQuery = this.props.query;
     delete urlQuery.embed;
-    const url = `/search?${queryString.stringify(urlQuery)}`;
-    window.open(prefixUrl(url), '_blank');
+    return `/search?${queryString.stringify(urlQuery)}`;
   };
 
   render() {
@@ -83,25 +83,24 @@ export class SearchTracePageImpl extends Component {
       maxTraceDuration,
       services,
       traceResults,
-      isEmbed,
+      embed,
       hideGraph,
-      disableComparision,
     } = this.props;
     const hasTraceResults = traceResults && traceResults.length > 0;
     const showErrors = errors && !loadingTraces;
     const showLogo = isHomepage && !hasTraceResults && !loadingTraces && !errors;
     return (
       <div>
         <Row>
-          {!isEmbed && (
+          {!embed && (
             <Col span={6} className="SearchTracePage--column">
               <div className="SearchTracePage--find">
                 <h2>Find Traces</h2>
                 {!loadingServices && services ? <SearchForm services={services} /> : <LoadingIndicator />}
               </div>
             </Col>
           )}
-          <Col span={18} className="SearchTracePage--column">
+          <Col span={!embed ? 18 : 24} className="SearchTracePage--column">
             {showErrors && (
               <div className="js-test-error-message">
                 <h2>There was an error querying for traces:</h2>
@@ -117,15 +116,14 @@ export class SearchTracePageImpl extends Component {
                 cohortRemoveTrace={cohortRemoveTrace}
                 diffCohort={diffCohort}
                 skipMessage={isHomepage}
-                onGoFullClicked={this.goFullView}
+                getSearchURL={this.getSearchURL}
                 traces={traceResults}
-                isEmbed={isEmbed}
+                embed={embed}
                 hideGraph={hideGraph}
-                disableComparision={disableComparision}
               />
             )}
             {showLogo &&
-              !isEmbed && (
+              !embed && (
                 <img
                   className="SearchTracePage--logo js-test-logo"
                   alt="presentation"
@@ -143,9 +141,8 @@ export class SearchTracePageImpl extends Component {
 SearchTracePageImpl.propTypes = {
   query: PropTypes.object,
   isHomepage: PropTypes.bool,
-  isEmbed: PropTypes.bool,
+  embed: PropTypes.bool,
   hideGraph: PropTypes.bool,
-  disableComparision: PropTypes.bool,
   // eslint-disable-next-line react/forbid-prop-types
   traceResults: PropTypes.array,
   diffCohort: PropTypes.array,
@@ -218,9 +215,7 @@ const stateServicesXformer = getLastXformCacher(stateServices => {
 // export to test
 export function mapStateToProps(state) {
   const query = queryString.parse(state.router.location.search);
-  const isEmbed = 'embed' in query;
-  const hideGraph = 'hideGraph' in query;
-  const disableComparision = 'disableComparision' in query;
+  const { hideGraph } = queryString.parse(state.router.location.search);
   const isHomepage = !Object.keys(query).length;
   const { traces, maxDuration, traceError, loadingTraces } = stateTraceXformer(state.trace);
   const diffCohort = stateTraceDiffXformer(state.trace, state.traceDiff);
@@ -237,9 +232,8 @@ export function mapStateToProps(state) {
   return {
     query,
     diffCohort,
-    isEmbed,
-    hideGraph,
-    disableComparision,
+    embed: isEmbed(state.router.location.search),
+    hideGraph: hideGraph !== undefined,
     isHomepage,
     loadingServices,
     loadingTraces,

packages/jaeger-ui/src/components/SearchTracePage/index.test.js

@@ -12,8 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import prefixUrl from '../../utils/prefix-url';
-
 jest.mock('redux-form', () => {
   function reduxForm() {
     return component => component;
@@ -50,8 +48,7 @@ describe('<SearchTracePage>', () => {
     traceResults = [{ traceID: 'a', spans: [], processes: {} }, { traceID: 'b', spans: [], processes: {} }];
     props = {
       traceResults,
-      isEmbed: false,
-      disableComparision: false,
+      embed: false,
       isHomepage: false,
       loadingServices: false,
       loadingTraces: false,
@@ -106,21 +103,14 @@ describe('<SearchTracePage>', () => {
   });
 
   it('hide SearchForm if is embed', () => {
-    wrapper.setProps({ isEmbed: true });
+    wrapper.setProps({ embed: true });
     expect(wrapper.find(SearchForm).length).toBe(0);
   });
 
   it('hide logo if is embed', () => {
-    wrapper.setProps({ isEmbed: true });
+    wrapper.setProps({ embed: true });
     expect(wrapper.find('.js-test-logo').length).toBe(0);
   });
-
-  it('open a window when goFullView is called', () => {
-    wrapper.setProps({ query: { embed: 'embed', service: 'jaeger-query' } });
-    global.open = jest.fn();
-    wrapper.instance().goFullView();
-    expect(global.open).toBeCalledWith(prefixUrl('/search?service=jaeger-query'), '_blank');
-  });
 });
 
 describe('mapStateToProps()', () => {
@@ -161,9 +151,8 @@ describe('mapStateToProps()', () => {
     expect(diffCohort[0].data.traceID).toBe(trace.traceID);
 
     expect(rest).toEqual({
-      isEmbed: false,
+      embed: false,
       hideGraph: false,
-      disableComparision: false,
       query: {},
       isHomepage: true,
       // the redux-form `formValueSelector` mock returns `null` for "sortBy"

packages/jaeger-ui/src/components/TracePage/TracePageHeader.css

@@ -19,26 +19,19 @@ limitations under the License.
   display: flex;
 }
 
-Divider {
-  color: red;
-}
-
 .TracePageHeader--titleRowEmbed {
   align-items: center;
   background-color: #ececec;
   border-bottom: 1px solid #d8d8d8;
   display: flex;
 }
 
-.TracePageHeader--title {
+.TracePageHeader--title,
+TracePageHeader--titleEmbed {
   margin: 0;
   padding: 0.25rem 0.5rem;
 }
 
-.TracePageHeader--titleEmbed {
-  margin: 0;
-  padding: 0.25rem 0.5rem;
-}
 .TracePageHeader--title:hover {
   background: #f5f5f5;
 }