Improve truncation of long values in tables

Instead of applying text truncation only to links in tables, apply it to all columns (with the exception of status icon + action items e.g. overflow menu). Update all tables to ensure tooltips available for truncated values, including the FormattedDuration component. A side effect of this is eliminating the horizontal scroll required for many of the wider tables (e.g. PipelineRuns) at medium screen sizes. This change also allows values to expand to use more space when available instead of truncating them at an arbitrarily chosen hardcoded width of 250px as previously. This means that more values are displayed in full instead of truncating them unnecessarily when the space is available. Since we're guaranteed to only have a single line of content in each cell, we can also tighten up the styles to align everything vertically.
tektoncd · Jan 17, 2020 · 2afb8bc · 2afb8bc
1 parent c1891bb
commit 2afb8bc
Show file tree

Hide file tree

Showing 15 changed files with 166 additions and 68 deletions.
diff --git a/packages/components/src/components/FormattedDuration/FormattedDuration.js b/packages/components/src/components/FormattedDuration/FormattedDuration.js
@@ -11,7 +11,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import React from 'react';
+import React, { Component } from 'react';
 import { defineMessages } from 'react-intl';
 import FormattedDuration from 'react-intl-formatted-duration';
 
@@ -38,13 +38,38 @@ defineMessages({
   }
 });
 
-const FormattedDurationWrapper = ({ milliseconds }) => {
-  return (
-    <FormattedDuration
-      seconds={milliseconds / 1000}
-      format="{days} {hours} {minutes} {seconds}"
-    />
-  );
-};
+export default class FormattedDurationWrapper extends Component {
+  state = { tooltip: '' };
 
-export default FormattedDurationWrapper;
+  componentDidMount() {
+    this.setState({
+      tooltip: this.durationNode && this.durationNode.textContent
+    });
+  }
+
+  componentDidUpdate() {
+    const duration = this.durationNode.textContent;
+    if (this.state.tooltip !== duration) {
+      this.setState({ // eslint-disable-line
+        tooltip: duration
+      });
+    }
+  }
+
+  render() {
+    const { milliseconds } = this.props;
+    return (
+      <span
+        ref={ref => {
+          this.durationNode = ref;
+        }}
+        title={this.state.tooltip}
+      >
+        <FormattedDuration
+          seconds={milliseconds / 1000}
+          format="{days} {hours} {minutes} {seconds}"
+        />
+      </span>
+    );
+  }
+}
diff --git a/packages/components/src/components/FormattedDuration/FormattedDuration.test.js b/packages/components/src/components/FormattedDuration/FormattedDuration.test.js
@@ -0,0 +1,30 @@
+/*
+Copyright 2020 The Tekton Authors
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from 'react';
+import { renderWithIntl, rerenderWithIntl } from '../../utils/test';
+import FormattedDuration from './FormattedDuration';
+
+describe('FormattedDuration', () => {
+  it('renders the duration and handles updates', () => {
+    const { getByText, getByTitle, rerender } = renderWithIntl(
+      <FormattedDuration milliseconds={1000} />
+    );
+    expect(getByText(/1 second/i)).toBeTruthy();
+    expect(getByTitle(/1 second/i)).toBeTruthy();
+
+    rerenderWithIntl(rerender, <FormattedDuration milliseconds={2000} />);
+    expect(getByText(/2 seconds/i)).toBeTruthy();
+    expect(getByTitle(/2 seconds/i)).toBeTruthy();
+  });
+});
diff --git a/packages/components/src/components/PipelineResources/PipelineResources.js b/packages/components/src/components/PipelineResources/PipelineResources.js
@@ -58,7 +58,7 @@ const PipelineResources = ({
       })
     },
     {
-      key: 'dropdown',
+      key: 'actions',
       header: ''
     }
   ];
@@ -80,17 +80,21 @@ const PipelineResources = ({
           {pipelineResourceName}
         </Link>
       ) : (
-        pipelineResourceName
+        <span title={pipelineResourceName}>{pipelineResourceName}</span>
+      ),
+      namespace: <span title={namespace}>{namespace}</span>,
+      type: (
+        <span title={pipelineResource.spec.type}>
+          {pipelineResource.spec.type}
+        </span>
       ),
-      namespace,
-      type: pipelineResource.spec.type,
       createdTime: (
         <FormattedDate
           date={pipelineResource.metadata.creationTimestamp}
           relative
         />
       ),
-      dropdown: (
+      actions: (
         <RunDropdown
           items={pipelineResourceActions}
           resource={pipelineResource}

diff --git a/packages/components/src/components/PipelineRuns/PipelineRuns.js b/packages/components/src/components/PipelineRuns/PipelineRuns.js
@@ -56,7 +56,7 @@ const PipelineRuns = ({
   pipelineRuns,
   pipelineRunActions
 }) => {
-  const initialHeaders = [
+  const headers = [
     {
       key: 'status',
       header: intl.formatMessage({
@@ -100,17 +100,10 @@ const PipelineRuns = ({
       })
     },
     {
-      key: 'dropdown',
+      key: 'actions',
       header: ''
     }
-  ];
-
-  const headers = [];
-  initialHeaders.forEach(header => {
-    if (header.key !== undefined) {
-      headers.push(header);
-    }
-  });
+  ].filter(Boolean);
 
   const pipelineRunsFormatted = pipelineRuns.map(pipelineRun => {
     const { annotations, creationTimestamp, namespace } = pipelineRun.metadata;
@@ -152,7 +145,7 @@ const PipelineRuns = ({
           {pipelineRunName}
         </Link>
       ) : (
-        pipelineRunName
+        <span title={pipelineRunName}>{pipelineRunName}</span>
       ),
       pipeline:
         pipelineRefName &&
@@ -163,7 +156,7 @@ const PipelineRuns = ({
         ) : (
           pipelineRefName
         )),
-      namespace: !hideNamespace && namespace,
+      namespace: !hideNamespace && <span title={namespace}>{namespace}</span>,
       status: (
         <div className="definition">
           <div
@@ -179,9 +172,7 @@ const PipelineRuns = ({
       createdTime: <FormattedDate date={creationTimestamp} relative />,
       duration,
       type: pipelineRunType,
-      dropdown: (
-        <RunDropdown items={pipelineRunActions} resource={pipelineRun} />
-      )
+      actions: <RunDropdown items={pipelineRunActions} resource={pipelineRun} />
     };
   });
 

diff --git a/packages/components/src/components/Table/Table.js b/packages/components/src/components/Table/Table.js
@@ -1,5 +1,5 @@
 /*
-Copyright 2019 The Tekton Authors
+Copyright 2019-2020 The Tekton Authors
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
@@ -121,7 +121,6 @@ const Table = props => {
                       return header.header ? (
                         <TableHeader
                           {...getHeaderProps({ header })}
-                          className="cellText"
                           key={header.key}
                         >
                           {header.header}
@@ -155,7 +154,11 @@ const Table = props => {
                           <TableSelectRow {...getSelectionProps({ row })} />
                         )}
                         {row.cells.map(cell => (
-                          <TableCell key={cell.id} id={cell.id}>
+                          <TableCell
+                            key={cell.id}
+                            id={cell.id}
+                            className={`cell-${cell.info.header}`}
+                          >
                             {cell.value}
                           </TableCell>
                         ))}

diff --git a/packages/components/src/components/Table/Table.scss b/packages/components/src/components/Table/Table.scss
@@ -18,14 +18,6 @@ limitations under the License.
   @import '~carbon-components/scss/components/data-table/data-table-core';
   @import '~carbon-components/scss/components/overflow-menu/overflow-menu';
 
-  a {
-    display: inline-block;
-    overflow: hidden;
-    text-overflow: ellipsis;
-    white-space: nowrap;
-    max-width: 250px;
-  }
-
   .bx--table-toolbar {
     background: transparent;
   }
@@ -40,8 +32,22 @@ limitations under the License.
     width: 100%;
   }
 
-  .bx--data-table thead th {
-    padding: 15;
+  .bx--data-table td {
+    &:not(.cell-actions):not(.cell-status) {
+      overflow: hidden;
+      text-overflow: ellipsis;
+      white-space: nowrap;
+      max-width: 10vw;
+      vertical-align: middle;
+    }
+
+    &.cell-status {
+      vertical-align: middle;
+    }
+
+    &.cell-status, &.cell-actions {
+      width: 40px;
+    }
   }
 
   .bx--data-table thead th {

diff --git a/packages/components/src/components/TaskRuns/TaskRuns.js b/packages/components/src/components/TaskRuns/TaskRuns.js
@@ -94,7 +94,7 @@ const TaskRuns = ({
       })
     },
     {
-      key: 'dropdown',
+      key: 'actions',
       header: ''
     }
   ];
@@ -130,7 +130,7 @@ const TaskRuns = ({
           {taskRunName}
         </Link>
       ) : (
-        taskRunName
+        <span title={taskRunName}>{taskRunName}</span>
       ),
       task: taskRefName ? (
         <Link
@@ -145,7 +145,11 @@ const TaskRuns = ({
       ) : (
         ''
       ),
-      namespace: taskRun.metadata.namespace,
+      namespace: (
+        <span title={taskRun.metadata.namespace}>
+          {taskRun.metadata.namespace}
+        </span>
+      ),
       status: (
         <div className="definition">
           <div
@@ -162,7 +166,7 @@ const TaskRuns = ({
         <FormattedDate date={taskRun.metadata.creationTimestamp} relative />
       ),
       duration,
-      dropdown: <RunDropdown items={taskRunActions} resource={taskRun} />
+      actions: <RunDropdown items={taskRunActions} resource={taskRun} />
     };
   });
 

diff --git a/packages/components/src/utils/test.js b/packages/components/src/utils/test.js
@@ -1,5 +1,5 @@
 /*
-Copyright 2019 The Tekton Authors
+Copyright 2019-2020 The Tekton Authors
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
@@ -43,12 +43,22 @@ export function renderWithRouter(
   };
 }
 
+export function wrapWithIntl(ui) {
+  return (
+    <IntlProvider locale="en" defaultLocale="en" messages={{}}>
+      {ui}
+    </IntlProvider>
+  );
+}
+
 export function renderWithIntl(ui) {
   return {
-    ...render(
-      <IntlProvider locale="en" defaultLocale="en" messages={{}}>
-        {ui}
-      </IntlProvider>
-    )
+    ...render(wrapWithIntl(ui))
+  };
+}
+
+export function rerenderWithIntl(rerender, ui) {
+  return {
+    ...rerender(wrapWithIntl(ui))
   };
 }
diff --git a/src/containers/EventListeners/EventListeners.js b/src/containers/EventListeners/EventListeners.js
@@ -138,7 +138,11 @@ export /* istanbul ignore next */ class EventListeners extends Component {
           {listener.metadata.name}
         </Link>
       ),
-      namespace: listener.metadata.namespace,
+      namespace: (
+        <span title={listener.metadata.namespace}>
+          {listener.metadata.namespace}
+        </span>
+      ),
       date: (
         <FormattedDate date={listener.metadata.creationTimestamp} relative />
       )

diff --git a/src/containers/Pipelines/Pipelines.js b/src/containers/Pipelines/Pipelines.js
@@ -80,7 +80,7 @@ export /* istanbul ignore next */ class Pipelines extends Component {
         })
       },
       {
-        key: 'link',
+        key: 'actions',
         header: ''
       }
     ];
@@ -98,11 +98,15 @@ export /* istanbul ignore next */ class Pipelines extends Component {
           {pipeline.metadata.name}
         </Link>
       ),
-      namespace: pipeline.metadata.namespace,
+      namespace: (
+        <span title={pipeline.metadata.namespace}>
+          {pipeline.metadata.namespace}
+        </span>
+      ),
       createdTime: (
         <FormattedDate date={pipeline.metadata.creationTimestamp} relative />
       ),
-      link: (
+      actions: (
         <Link
           to={urls.rawCRD.byNamespace({
             namespace: pipeline.metadata.namespace,

diff --git a/src/containers/Secrets/Secrets.js b/src/containers/Secrets/Secrets.js
@@ -184,13 +184,16 @@ export /* istanbul ignore next */ class Secrets extends Component {
           }
         });
       });
+      const serviceAccountsString = serviceAccountsWithSecret.join(', ');
       const formattedSecret = {
-        annotations,
+        annotations: <span title={annotations}>{annotations}</span>,
         id: `${secret.namespace}:${secret.name}`,
-        name: secret.name,
-        namespace: secret.namespace,
+        name: <span title={secret.name}>{secret.name}</span>,
+        namespace: <span title={secret.namespace}>{secret.namespace}</span>,
         created: <FormattedDate date={secret.creationTimestamp} relative />,
-        serviceAccounts: serviceAccountsWithSecret.join(', ')
+        serviceAccounts: (
+          <span title={serviceAccountsString}>{serviceAccountsString}</span>
+        )
       };
 
       return formattedSecret;