Styling tweaks to warnings in span details

Signed-off-by: Joe Farro <joef@uber.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
vvvprabhakar · May 16, 2019 · 4e9b19c · 4e9b19c
1 parent 11b36ff
commit 4e9b19c
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 28 deletions.
diff --git a/...s/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianText.test.js b/...s/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianText.test.js
@@ -46,10 +46,10 @@ describe('<AccordianText>', () => {
     expect(header.text()).toBe(props.label);
   });
 
-  it('renders the table instead of the summary when it is expanded', () => {
+  it('renders the content when it is expanded', () => {
     wrapper.setProps({ isOpen: true });
-    const table = wrapper.find(TextList);
-    expect(table.length).toBe(1);
-    expect(table.prop('data')).toBe(warnings);
+    const content = wrapper.find(TextList);
+    expect(content.length).toBe(1);
+    expect(content.prop('data')).toBe(warnings);
   });
 });
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianText.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianText.tsx
@@ -24,15 +24,16 @@ import './AccordianText.css';
 type AccordianTextProps = {
   className?: string | TNil;
   data: string[];
+  headerClassName?: string | TNil;
   highContrast?: boolean;
   interactive?: boolean;
   isOpen: boolean;
-  label: string;
+  label: React.ReactNode;
   onToggle?: null | (() => void);
 };
 
 export default function AccordianText(props: AccordianTextProps) {
-  const { className, data, highContrast, interactive, isOpen, label, onToggle } = props;
+  const { className, data, headerClassName, highContrast, interactive, isOpen, label, onToggle } = props;
   const isEmpty = !Array.isArray(data) || !data.length;
   const iconCls = cx('u-align-icon', { 'AccordianKeyValues--emptyIcon': isEmpty });
   let arrow: React.ReactNode | null = null;
@@ -46,11 +47,12 @@ export default function AccordianText(props: AccordianTextProps) {
     };
   }
   return (
-    <div className={cx(className, 'u-tx-ellipsis')}>
+    <div className={className || ''}>
       <div
-        className={cx('AccordianText--header', {
+        className={cx('AccordianText--header', headerClassName, {
           'is-empty': isEmpty,
           'is-high-contrast': highContrast,
+          'is-open': isOpen,
         })}
         {...headerProps}
       >

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/TextList.css b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/TextList.css
@@ -15,7 +15,6 @@ limitations under the License.
 */
 
 .TextList {
-  background: #fff;
   max-height: 450px;
   overflow: auto;
 }

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.css b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.css
@@ -41,20 +41,23 @@ limitations under the License.
   color: #333;
 }
 .AccordianWarnings {
-  border: 1px solid #d8d8d8;
+  background: #fafafa;
+  border: 1px solid #e4e4e4;
+  margin-bottom: 0.25rem;
 }
-.AccordianWarnings > .AccordianText--header > strong {
-  color: #d36c08;
-}
-
-.AccordianWarnings > .AccordianText--header {
+.AccordianWarnings--header {
   background: #fff7e6;
   padding: 0.25rem 0.5rem;
 }
 
-.SpanDetail--warnIcon {
-  background: transparent;
-  color: #ffa500;
-  font-size: 1em;
-  margin-right: 0.2rem;
+.AccordianWarnings--header:hover {
+  background: #ffe7ba;
+}
+
+.AccordianWarnings--header.is-open {
+  border-bottom: 1px solid #e8e8e8;
+}
+
+.AccordianWarnings--label {
+  color: #d36c08;
 }
diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.test.js
@@ -20,7 +20,6 @@ import { shallow } from 'enzyme';
 
 import AccordianKeyValues from './AccordianKeyValues';
 import AccordianLogs from './AccordianLogs';
-import AccordianText from './AccordianText';
 import DetailState from './DetailState';
 import SpanDetail from './index';
 import { formatDuration } from '../utils';
@@ -125,11 +124,9 @@ describe('<SpanDetail>', () => {
   });
 
   it('renders the warnings', () => {
-    const target = (
-      <AccordianText data={span.warnings} label="Warnings" isOpen={detailState.isWarningsOpen} />
-    );
-    expect(wrapper.containsMatchingElement(target)).toBe(true);
-    wrapper.find({ data: span.warnings }).simulate('toggle');
+    const warningElm = wrapper.find({ data: span.warnings });
+    expect(warningElm.length).toBe(1);
+    warningElm.simulate('toggle');
     expect(props.warningsToggle).toHaveBeenLastCalledWith(span.spanID);
   });
 

diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.tsx
@@ -119,8 +119,9 @@ export default function SpanDetail(props: SpanDetailProps) {
         {warnings &&
           warnings.length > 0 && (
             <AccordianText
-              className="AccordianWarnings ub-mb1"
-              label="Warnings"
+              className="AccordianWarnings"
+              headerClassName="AccordianWarnings--header"
+              label={<span className="AccordianWarnings--label">Warnings</span>}
               data={warnings}
               isOpen={isWarningsOpen}
               onToggle={() => warningsToggle(spanID)}