Browse Source

fix(discover) Improve auto-layout of discover results (#16805)

Improve the auto-layout by using the `minmax()` grid function to get
grid colums fitting into 100% for the initial load. This will also allow
the table to resize if the window is resized.

Once columns are sized then their width in the grid become fixed width
persisting through browser resizing.
Mark Story 5 years ago
parent
commit
d692554732

+ 11 - 55
src/sentry/static/sentry/app/components/gridEditable/index.tsx

@@ -36,18 +36,7 @@ import {
 import GridHeadCell from './gridHeadCell';
 import GridModalEditColumn from './gridModalEditColumn';
 
-import {
-  COL_WIDTH_UNDEFINED,
-  COL_WIDTH_MIN,
-  COL_WIDTH_DEFAULT,
-  COL_WIDTH_BOOLEAN,
-  COL_WIDTH_DATETIME,
-  COL_WIDTH_NUMBER,
-  COL_WIDTH_STRING,
-  COL_WIDTH_STRING_LONG,
-  COL_WIDTH_STRING_SHORT,
-  ColResizeMetadata,
-} from './utils';
+import {COL_WIDTH_UNDEFINED, ColResizeMetadata} from './utils';
 
 type GridEditableProps<DataRow, ColumnKey> = {
   onToggleEdit?: (nextValue: boolean) => void;
@@ -301,11 +290,7 @@ class GridEditable<
       width: metadata.columnWidth + widthChange,
     };
 
-    this.setGridTemplateColumns(
-      this.props.columnOrder,
-      metadata.columnIndex,
-      metadata.columnWidth + e.clientX - metadata.cursorX
-    );
+    this.setGridTemplateColumns(nextColumnOrder);
   }
 
   /**
@@ -318,42 +303,21 @@ class GridEditable<
   /**
    * Set the CSS for Grid Column
    */
-  setGridTemplateColumns(
-    columnOrder: GridColumnOrder[],
-    columnIndex: number = -1,
-    columnWidth: number = 0
-  ) {
+  setGridTemplateColumns(columnOrder: GridColumnOrder[]) {
     const grid = this.refGrid.current;
     if (!grid) {
       return;
     }
-
     const prependColumns = this.props.grid.prependColumnWidths || [];
-    let sumWidth = prependColumns.reduce((acc, item) => acc + parseInt(item, 10), 0);
-
-    const columnWidths = prependColumns.concat(
-      columnOrder.map((c, i) => {
-        let width =
-          i === columnIndex // Case 1: Resize, then draw a specific column
-            ? columnWidth
-            : !c.width || isNaN(c.width) // Case 2: Draw a column with no width
-            ? COL_WIDTH_DEFAULT
-            : c.width; // Case 3: Draw a column with width
-
-        width = Math.max(COL_WIDTH_MIN, width);
-        sumWidth += width;
-
-        return `${width}px`;
-      })
-    );
-
-    // If columns are smaller than grid, let the last column fill the remaining
-    // blank space on the right of the grid
-    if (sumWidth < grid.offsetWidth) {
-      columnWidths[columnWidths.length - 1] = '1fr';
-    }
+    const prepend = prependColumns.join(' ');
+    const widths = columnOrder.map(item => {
+      if (item.width !== COL_WIDTH_UNDEFINED) {
+        return `${item.width}px`;
+      }
+      return 'minmax(50px, auto)';
+    });
 
-    grid.style.gridTemplateColumns = columnWidths.join(' ');
+    grid.style.gridTemplateColumns = `${prepend} ${widths.join(' ')}`;
   }
 
   renderHeaderButtons() {
@@ -585,14 +549,6 @@ class GridEditable<
 export default GridEditable;
 export {
   COL_WIDTH_UNDEFINED,
-  COL_WIDTH_MIN,
-  COL_WIDTH_DEFAULT,
-  COL_WIDTH_BOOLEAN,
-  COL_WIDTH_DATETIME,
-  COL_WIDTH_NUMBER,
-  COL_WIDTH_STRING,
-  COL_WIDTH_STRING_LONG,
-  COL_WIDTH_STRING_SHORT,
   GridColumn,
   GridColumnHeader,
   GridColumnOrder,

+ 3 - 2
src/sentry/static/sentry/app/components/gridEditable/styles.tsx

@@ -99,14 +99,14 @@ export const Grid = styled('table')`
   display: grid;
 
   /* Overwritten by GridEditable.setGridTemplateColumns */
-  grid-template-columns: repeat(auto-fill, 1fr);
+  grid-template-columns: repeat(auto-fill, minmax(50px, auto));
 
   box-sizing: border-box;
   border-collapse: collapse;
   margin: 0;
 
-  overflow-x: scroll;
   z-index: ${Z_INDEX_GRID};
+  overflow-x: scroll;
 `;
 export const GridRow = styled('tr')`
   display: contents;
@@ -135,6 +135,7 @@ export const GridHeadCell = styled('th')`
   min-width: 0;
   height: ${GRID_HEAD_ROW_HEIGHT}px;
   border-right: 1px solid transparent;
+  border-left: 1px solid transparent;
   background-color: ${p => p.theme.offWhite};
   border-bottom: 1px solid ${p => p.theme.borderDark};
 

+ 1 - 12
src/sentry/static/sentry/app/components/gridEditable/utils.tsx

@@ -1,16 +1,5 @@
+// Auto layout width.
 export const COL_WIDTH_UNDEFINED = -1;
-export const COL_WIDTH_MIN = 100;
-
-// Default width if it wasn't specified
-export const COL_WIDTH_DEFAULT = 300;
-
-// Starting defaults if column values are known
-export const COL_WIDTH_BOOLEAN = COL_WIDTH_MIN;
-export const COL_WIDTH_DATETIME = 200;
-export const COL_WIDTH_NUMBER = COL_WIDTH_MIN;
-export const COL_WIDTH_STRING = COL_WIDTH_DEFAULT;
-export const COL_WIDTH_STRING_LONG = 500;
-export const COL_WIDTH_STRING_SHORT = 200;
 
 // Store state at the start of "resize" action
 export type ColResizeMetadata = {

+ 5 - 53
src/sentry/static/sentry/app/views/eventsV2/utils.tsx

@@ -19,16 +19,7 @@ import {TagSegment} from 'app/components/tagDistributionMeter';
 import {URL_PARAM} from 'app/constants/globalSelectionHeader';
 import {disableMacros} from 'app/views/discover/result/utils';
 import {appendTagCondition} from 'app/utils/queryString';
-import {
-  COL_WIDTH_UNDEFINED,
-  COL_WIDTH_DEFAULT,
-  COL_WIDTH_BOOLEAN,
-  COL_WIDTH_DATETIME,
-  COL_WIDTH_NUMBER,
-  COL_WIDTH_STRING,
-  COL_WIDTH_STRING_SHORT,
-  COL_WIDTH_STRING_LONG,
-} from 'app/components/gridEditable';
+import {COL_WIDTH_UNDEFINED} from 'app/components/gridEditable';
 
 import {
   AGGREGATE_ALIASES,
@@ -70,7 +61,7 @@ export function explodeField(field: FieldType): Column {
   return {
     aggregation: results.aggregation,
     field: results.field,
-    width: field.width || COL_WIDTH_DEFAULT,
+    width: field.width || COL_WIDTH_UNDEFINED,
   };
 }
 
@@ -157,42 +148,6 @@ export type MetaType = {
   [key: string]: FieldTypes;
 };
 
-export function getDefaultWidth(key: Aggregation | Field): number {
-  if (AGGREGATIONS[key]) {
-    return COL_WIDTH_NUMBER;
-  }
-
-  // Some columns have specific lengths due to longer content.
-  switch (key) {
-    case 'title':
-      return COL_WIDTH_STRING_LONG + 50;
-    case 'url':
-    case 'transaction':
-      return COL_WIDTH_STRING_LONG;
-    case 'user':
-    case 'project':
-      return COL_WIDTH_STRING_SHORT;
-    case 'event.type':
-      return COL_WIDTH_STRING_SHORT - 50;
-    default:
-      break;
-  }
-
-  switch (FIELDS[key]) {
-    case 'string':
-      return COL_WIDTH_STRING;
-    case 'boolean':
-      return COL_WIDTH_BOOLEAN;
-    case 'number':
-    case 'duration':
-      return COL_WIDTH_NUMBER;
-    case 'never': // never is usually a timestamp
-      return COL_WIDTH_DATETIME;
-    default:
-      return COL_WIDTH_DEFAULT;
-  }
-}
-
 /**
  * Get the field renderer for the named field and metadata
  *
@@ -241,13 +196,13 @@ const TEMPLATE_TABLE_COLUMN: TableColumn<React.ReactText> = {
   aggregation: '',
   field: '',
   name: '',
-  width: COL_WIDTH_DEFAULT,
+  width: COL_WIDTH_UNDEFINED,
 
   type: 'never',
   isDragging: false,
   isSortable: false,
 
-  eventViewField: Object.freeze({field: '', width: COL_WIDTH_DEFAULT}),
+  eventViewField: Object.freeze({field: '', width: COL_WIDTH_UNDEFINED}),
 };
 
 export function decodeColumnOrder(
@@ -271,10 +226,7 @@ export function decodeColumnOrder(
     }
     column.key = col.aggregationField;
     column.type = column.aggregation ? 'number' : FIELDS[column.field];
-    column.width =
-      col.width && col.width !== COL_WIDTH_UNDEFINED
-        ? col.width
-        : getDefaultWidth(aggregationField[0]);
+    column.width = col.width;
 
     column.name = column.key;
     column.isSortable = AGGREGATIONS[column.aggregation]

+ 8 - 8
tests/js/spec/views/eventsV2/utils.spec.jsx

@@ -310,8 +310,8 @@ describe('getExpandedResults()', function() {
     let result = getExpandedResults(view, {}, {});
 
     expect(result.fields).toEqual([
-      {field: 'id', width: 300}, // expect count() to be converted to id
-      {field: 'timestamp', width: 300},
+      {field: 'id', width: -1}, // expect count() to be converted to id
+      {field: 'timestamp', width: -1},
       {field: 'title'},
       {field: 'custom_tag'},
     ]);
@@ -333,8 +333,8 @@ describe('getExpandedResults()', function() {
     result = getExpandedResults(view, {}, {});
 
     expect(result.fields).toEqual([
-      {field: 'id', width: 300}, // expect count() to be converted to id
-      {field: 'timestamp', width: 300},
+      {field: 'id', width: -1}, // expect count() to be converted to id
+      {field: 'timestamp', width: -1},
       {field: 'title'},
       {field: 'custom_tag'},
     ]);
@@ -367,10 +367,10 @@ describe('getExpandedResults()', function() {
     result = getExpandedResults(view, {}, {});
 
     expect(result.fields).toEqual([
-      {field: 'timestamp', width: 300},
-      {field: 'id', width: 300},
+      {field: 'timestamp', width: -1},
+      {field: 'id', width: -1},
       {field: 'title'},
-      {field: 'transaction.duration', width: 300},
+      {field: 'transaction.duration', width: -1},
       {field: 'custom_tag'},
       {field: 'title'},
     ]);
@@ -451,7 +451,7 @@ describe('explodeField', function() {
     expect(explodeField({field: 'foobar'})).toEqual({
       aggregation: '',
       field: 'foobar',
-      width: 300,
+      width: -1,
     });
 
     // has width