Table: Fix text wrapping applying to wrong field (#93707)

codeincarnate · Develer · web-flow · commit d7ee3ea0861e · 2024-10-17T20:33:59.000+02:00
* Fix text wrap
---------

Co-authored-by: Ihor Yeromin &lt;yeryomin.igor@gmail.com&gt;
diff --git a/packages/grafana-ui/src/components/Table/RowsList.tsx b/packages/grafana-ui/src/components/Table/RowsList.tsx
@@ -53,6 +53,7 @@ interface RowsListProps {
   initialRowIndex?: number;
   headerGroups: HeaderGroup[];
   longestField?: Field;
+  textWrapField?: Field;
 }
 
 export const RowsList = (props: RowsListProps) => {
@@ -78,6 +79,7 @@ export const RowsList = (props: RowsListProps) => {
     initialRowIndex = undefined,
     headerGroups,
     longestField,
+    textWrapField,
   } = props;
 
   const [rowHighlightIndex, setRowHighlightIndex] = useState<number | undefined>(initialRowIndex);
@@ -232,7 +234,7 @@ export const RowsList = (props: RowsListProps) => {
   );
 
   let rowBg: Function | undefined = undefined;
-  let textWrapField: Field | undefined = undefined;
+  let textWrapFinal: Field | undefined;
   for (const field of data.fields) {
     // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
     const fieldOptions = field.config.custom as TableFieldOptions;
@@ -250,16 +252,10 @@ export const RowsList = (props: RowsListProps) => {
       };
     }
 
-    if (
-      cellOptionsExist &&
-      (fieldOptions.cellOptions.type === TableCellDisplayMode.Auto ||
-        fieldOptions.cellOptions.type === TableCellDisplayMode.ColorBackground ||
-        fieldOptions.cellOptions.type === TableCellDisplayMode.ColorText) &&
-      fieldOptions.cellOptions.wrapText
-    ) {
-      textWrapField = field;
+    if (textWrapField !== undefined) {
+      textWrapFinal = textWrapField;
     } else if (longestField !== undefined) {
-      textWrapField = longestField;
+      textWrapFinal = longestField;
     }
   }
 
@@ -288,16 +284,17 @@ export const RowsList = (props: RowsListProps) => {
       }
 
       // If there's a text wrapping field we set the height of it here
-      if (textWrapField) {
+      if (textWrapFinal) {
         const visibleFields = data.fields.filter((field) => !Boolean(field.config.custom?.hidden));
-        const seriesIndex = visibleFields.findIndex((field) => field.name === textWrapField.name);
+        const seriesIndex = visibleFields.findIndex((field) => field.name === textWrapFinal.name);
         const pxLineHeight = theme.typography.body.lineHeight * theme.typography.fontSize;
         const bbox = guessTextBoundingBox(
-          textWrapField.values[index],
+          textWrapFinal.values[row.index],
           headerGroups[0].headers[seriesIndex],
           osContext,
           pxLineHeight,
-          tableStyles.rowHeight
+          tableStyles.rowHeight,
+          tableStyles.cellPadding
         );
         style.height = bbox.height;
       }
@@ -335,7 +332,7 @@ export const RowsList = (props: RowsListProps) => {
               frame={data}
               rowStyled={rowBg !== undefined}
               rowExpanded={rowExpanded}
-              textWrapped={textWrapField !== undefined}
+              textWrapped={textWrapFinal !== undefined}
               height={Number(style.height)}
             />
           ))}
@@ -354,7 +351,7 @@ export const RowsList = (props: RowsListProps) => {
       rows,
       tableState.expanded,
       tableStyles,
-      textWrapField,
+      textWrapFinal,
       theme.components.table.rowSelected,
       theme.typography.fontSize,
       theme.typography.body.lineHeight,
@@ -374,16 +371,17 @@ export const RowsList = (props: RowsListProps) => {
       return getExpandedRowHeight(nestedDataField, row.index, tableStyles);
     }
 
-    if (textWrapField) {
+    if (textWrapFinal) {
       const visibleFields = data.fields.filter((field) => !Boolean(field.config.custom?.hidden));
-      const seriesIndex = visibleFields.findIndex((field) => field.name === textWrapField.name);
+      const seriesIndex = visibleFields.findIndex((field) => field.name === textWrapFinal.name);
       const pxLineHeight = theme.typography.fontSize * theme.typography.body.lineHeight;
       return guessTextBoundingBox(
-        textWrapField.values[index],
+        textWrapFinal.values[row.index],
         headerGroups[0].headers[seriesIndex],
         osContext,
         pxLineHeight,
-        tableStyles.rowHeight
+        tableStyles.rowHeight,
+        tableStyles.cellPadding
       ).height;
     }
 
@@ -401,22 +399,29 @@ export const RowsList = (props: RowsListProps) => {
   // Key the virtualizer for expanded rows
   const expandedKey = Object.keys(tableState.expanded).join('|');
 
+  // It's a hack for text wrapping.
+  // VariableSizeList component didn't know that we manually set row height.
+  // So we need to reset the list when the rows high changes.
+  useEffect(() => {
+    if (listRef.current) {
+      listRef.current.resetAfterIndex(0);
+    }
+  }, [rows, listRef]);
+
   return (
-    <>
-      <CustomScrollbar onScroll={handleScroll} hideHorizontalTrack={true} scrollTop={scrollTop}>
-        <VariableSizeList
-          // This component needs an unmount/remount when row height, page changes, or expanded rows change
-          key={`${rowHeight}${pageIndex}${expandedKey}`}
-          height={listHeight}
-          itemCount={itemCount}
-          itemSize={getItemSize}
-          width={'100%'}
-          ref={listRef}
-          style={{ overflow: undefined }}
-        >
-          {({ index, style }) => RenderRow({ index, style, rowHighlightIndex })}
-        </VariableSizeList>
-      </CustomScrollbar>
-    </>
+    <CustomScrollbar onScroll={handleScroll} hideHorizontalTrack={true} scrollTop={scrollTop}>
+      <VariableSizeList
+        // This component needs an unmount/remount when row height, page changes, or expanded rows change
+        key={`${rowHeight}${pageIndex}${expandedKey}`}
+        height={listHeight}
+        itemCount={itemCount}
+        itemSize={getItemSize}
+        width={'100%'}
+        ref={listRef}
+        style={{ overflow: undefined }}
+      >
+        {({ index, style }) => RenderRow({ index, style, rowHighlightIndex })}
+      </VariableSizeList>
+    </CustomScrollbar>
   );
 };
diff --git a/packages/grafana-ui/src/components/Table/Table.tsx b/packages/grafana-ui/src/components/Table/Table.tsx
@@ -10,7 +10,7 @@ import {
 } from 'react-table';
 import { VariableSizeList } from 'react-window';
 
-import { FieldType, ReducerID, getRowUniqueId } from '@grafana/data';
+import { FieldType, ReducerID, getRowUniqueId, getFieldMatcher } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
 import { TableCellHeight } from '@grafana/schema';
 
@@ -297,7 +297,24 @@ export const Table = memo((props: Props) => {
   }
 
   // Try to determine the longet field
+  // TODO: do we wrap only one field?
+  // What if there are multiple fields with long text?
   const longestField = guessLongestField(fieldConfig, data);
+  let textWrapField = undefined;
+  if (fieldConfig !== undefined) {
+    data.fields.forEach((field) => {
+      fieldConfig.overrides.forEach((override) => {
+        const matcher = getFieldMatcher(override.matcher);
+        if (matcher(field, data, [data])) {
+          for (const property of override.properties) {
+            if (property.id === 'custom.cellOptions' && property.value.wrapText) {
+              textWrapField = field;
+            }
+          }
+        }
+      });
+    });
+  }
 
   return (
     <div
@@ -337,6 +354,7 @@ export const Table = memo((props: Props) => {
                 enableSharedCrosshair={enableSharedCrosshair}
                 initialRowIndex={initialRowIndex}
                 longestField={longestField}
+                textWrapField={textWrapField}
               />
             </div>
           ) : (
diff --git a/packages/grafana-ui/src/components/Table/utils.test.ts b/packages/grafana-ui/src/components/Table/utils.test.ts
@@ -77,7 +77,7 @@ function getWrappableData(numRecords: number) {
   // this case so we simply leave it as zero
   for (let i = 0; i < numRecords; i++) {
     data.fields[0].values[i] = 0;
-    data.fields[1].values[i] = faker.lorem.paragraphs(9);
+    data.fields[1].values[i] = faker.lorem.paragraphs(6);
     data.fields[2].values[i] = faker.lorem.paragraphs(11);
   }
 
@@ -546,7 +546,7 @@ describe('Table utils', () => {
 
   describe('guessLongestField', () => {
     // FLAKY TEST - https://drone.grafana.net/grafana/grafana/201232/1/5
-    it.skip('should guess the longest field correct if there are few records', () => {
+    it.skip('should guess the longest field correctly if there are few records', () => {
       const data = getWrappableData(10);
       const config = {
         defaults: {
diff --git a/packages/grafana-ui/src/components/Table/utils.ts b/packages/grafana-ui/src/components/Table/utils.ts
@@ -644,11 +644,13 @@ export function guessTextBoundingBox(
   headerGroup: HeaderGroup,
   osContext: OffscreenCanvasRenderingContext2D | null,
   lineHeight: number,
-  defaultRowHeight: number
+  defaultRowHeight: number,
+  padding = 0
 ) {
   const width = Number(headerGroup?.width ?? 300);
   const LINE_SCALE_FACTOR = 1.17;
   const LOW_LINE_PAD = 42;
+  const PADDING = padding * 2;
 
   if (osContext !== null && typeof text === 'string') {
     const words = text.split(/\s/);
@@ -662,7 +664,7 @@ export function guessTextBoundingBox(
       const currentWord = words[i];
       let lineWidth = osContext.measureText(currentLine + ' ' + currentWord).width;
 
-      if (lineWidth < width) {
+      if (lineWidth < width - PADDING) {
         currentLine += ' ' + currentWord;
         wordCount++;
       } else {
@@ -695,6 +697,7 @@ export function guessTextBoundingBox(
     } else {
       height = lineNumber * lineHeight + LOW_LINE_PAD;
     }
+    height += PADDING;
 
     return { width, height };
   }
