fix: simplyfy html structure for text highlight (#1187)

* fix: simplyfy html structure for text highlight move text highlight to a separate level of spans inside the labelText remove the need check the text for direction (dir="auto" works) makes it easier to style and less affected by other styling * fix: hide correct end of text for text overflow
qlik-oss · Mar 24, 2023 · 64a3b20 · 64a3b20
1 parent b13e542
commit 64a3b20
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 129 deletions.
diff --git a/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/ListBoxRowColumn.jsx b/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/ListBoxRowColumn.jsx
@@ -17,7 +17,6 @@ import Histogram from './components/Histogram';
 import Frequency from './components/Frequency';
 import ItemGrid from './components/ItemGrid';
 import getCellFromPages from './helpers/get-cell-from-pages';
-import rtlUtil from '../../../../utils/rtl-util';
 
 function RowColumn({ index, rowIndex, columnIndex, style, data }) {
   const {
@@ -137,7 +136,6 @@ function RowColumn({ index, rowIndex, columnIndex, style, data }) {
   const isGridCol = dataLayout === 'grid' && layoutOrder === 'column';
 
   const label = cell?.qText ?? '';
-  const textDirection = rtlUtil.detectTextDirection(label);
   // Search highlights. Split up labelText span into several and add the highlighted class to matching sub-strings.
 
   let labels;
@@ -232,7 +230,6 @@ function RowColumn({ index, rowIndex, columnIndex, style, data }) {
               isGridCol={isGridCol}
               isSingleSelect={isSingleSelect}
               valueTextAlign={valueTextAlign}
-              textDirection={textDirection}
             />
           ) : (
             <Field

diff --git a/...src/components/listbox/components/ListBoxRowColumn/__tests__/list-box-row-column.test.jsx b/...src/components/listbox/components/ListBoxRowColumn/__tests__/list-box-row-column.test.jsx
@@ -616,9 +616,12 @@ describe('<ListBoxRowColumn />', () => {
       );
       const testInstance = testRenderer.root;
       const types = testInstance.findAllByType(Typography);
-      expect(types[0].props.children.props.children).toBe('nebula.js');
-      expect(types[0].props.className.includes('highlighted')).toBe(true);
-      expect(types[1].props.children.props.children).toBe(' ftw');
+      expect(types).toHaveLength(1);
+      const spans = types[0].props.children.props.children;
+      expect(spans).toHaveLength(2);
+      expect(spans[0].props.children).toBe('nebula.js');
+      expect(spans[0].props.className.includes('highlighted')).toBe(true);
+      expect(spans[1].props.children).toBe(' ftw');
       await testRenderer.unmount();
     });
 
@@ -662,9 +665,12 @@ describe('<ListBoxRowColumn />', () => {
       );
       const testInstance = testRenderer.root;
       const types = testInstance.findAllByType(Typography);
-      expect(types[0].props.children.props.children).toBe('nebula.js ');
-      expect(types[1].props.children.props.children).toBe('ftw');
-      expect(types[1].props.className.includes('highlighted')).toBe(true);
+      expect(types).toHaveLength(1);
+      const spans = types[0].props.children.props.children;
+      expect(spans).toHaveLength(2);
+      expect(spans[0].props.children).toBe('nebula.js ');
+      expect(spans[1].props.children).toBe('ftw');
+      expect(spans[1].props.className.includes('highlighted')).toBe(true);
       // TODO: MUIv5 - no idea why this breaks
       // const hits = testInstance.findAllByProps({ className: 'RowColumn-highlighted' });
       // expect(hits).to.have.length(2);
@@ -711,10 +717,13 @@ describe('<ListBoxRowColumn />', () => {
       );
       const testInstance = testRenderer.root;
       const types = testInstance.findAllByType(Typography);
-      expect(types[0].props.children.props.children).toBe('nebula.js ftw ');
-      expect(types[1].props.children.props.children).toBe('yeah');
-      expect(types[1].props.className.includes('RowColumn-highlighted')).toBe(true);
-      expect(types[2].props.children.props.children).toBe(' buddy');
+      expect(types).toHaveLength(1);
+      const spans = types[0].props.children.props.children;
+      expect(spans).toHaveLength(3);
+      expect(spans[0].props.children).toBe('nebula.js ftw ');
+      expect(spans[1].props.children).toBe('yeah');
+      expect(spans[1].props.className.includes('RowColumn-highlighted')).toBe(true);
+      expect(spans[2].props.children).toBe(' buddy');
       await testRenderer.unmount();
     });
 
@@ -799,10 +808,13 @@ describe('<ListBoxRowColumn />', () => {
       // const cells = testInstance.findAllByProps({ className: 'RowColumn-highlighted' });
       // expect(cells).to.have.length(2);
       const types = testInstance.findAllByType(Typography);
-      expect(types[1].props.children.props.children).toBe('nebula.js ftw ');
-      expect(types[2].props.children.props.children).toBe('yeah');
-      expect(types[2].props.className.includes('RowColumn-highlighted')).toBe(true);
-      expect(types[3].props.children.props.children).toBe(' buddy');
+      expect(types).toHaveLength(2);
+      const spans = types[1].props.children.props.children;
+      expect(spans).toHaveLength(3);
+      expect(spans[0].props.children).toBe('nebula.js ftw ');
+      expect(spans[1].props.children).toBe('yeah');
+      expect(spans[1].props.className.includes('RowColumn-highlighted')).toBe(true);
+      expect(spans[2].props.children).toBe(' buddy');
       await testRenderer.unmount();
     });
   });

diff --git a/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/CheckboxField.jsx b/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/CheckboxField.jsx
@@ -17,7 +17,6 @@ function CheckboxField({
   isGridCol,
   showGray,
   isSingleSelect,
-  highlighted,
   checkboxes,
   valueTextAlign,
 }) {
@@ -46,7 +45,6 @@ function CheckboxField({
         <LabelTag
           label={label}
           color={color}
-          highlighted={highlighted}
           dense={dense}
           showGray={showGray}
           checkboxes={checkboxes}

diff --git a/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/Field.jsx b/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/Field.jsx
@@ -13,7 +13,6 @@ function Field({
   isGridCol,
   showGray,
   isSingleSelect,
-  highlighted,
   checkboxes,
   valueTextAlign,
 }) {
@@ -29,15 +28,13 @@ function Field({
       isGridCol={isGridCol}
       showGray={showGray}
       isSingleSelect={isSingleSelect}
-      highlighted={highlighted}
       checkboxes={checkboxes}
       valueTextAlign={valueTextAlign}
     />
   ) : (
     <ValueField
       label={label}
       color={color}
-      highlighted={false}
       dense={dense}
       showGray
       checkboxes={checkboxes}

diff --git a/...nucleus/src/components/listbox/components/ListBoxRowColumn/components/FieldWithRanges.jsx b/...nucleus/src/components/listbox/components/ListBoxRowColumn/components/FieldWithRanges.jsx
@@ -1,25 +1,15 @@
 import React from 'react';
 import CheckboxField from './CheckboxField';
 import ValueField from './ValueField';
+import classes from '../helpers/classes';
 
-function LabelsWithRanges({ labels, dense, showGray, checkboxes, textDirection }) {
-  return (
-    <div dir={textDirection}>
-      {labels.map(([label, highlighted], index) => {
-        const key = `${index}`;
-        return (
-          <ValueField
-            label={label}
-            key={key}
-            highlighted={highlighted}
-            dense={dense}
-            showGray={showGray}
-            checkboxes={checkboxes}
-          />
-        );
-      })}
-    </div>
-  );
+function LabelsWithRanges({ labels, dense, showGray, checkboxes }) {
+  const text = labels.map(([label, highlighted], index) => (
+    <span id={index} className={highlighted ? classes.highlighted : ''}>
+      {label}
+    </span>
+  ));
+  return <ValueField label={text} dense={dense} showGray={showGray} checkboxes={checkboxes} />;
 }
 
 function FieldWithRanges({
@@ -35,17 +25,8 @@ function FieldWithRanges({
   isGridCol,
   isSingleSelect,
   valueTextAlign,
-  textDirection,
 }) {
-  const LWR = (
-    <LabelsWithRanges
-      labels={labels}
-      dense={dense}
-      showGray={showGray}
-      checkboxes={checkboxes}
-      textDirection={textDirection}
-    />
-  );
+  const LWR = <LabelsWithRanges labels={labels} dense={dense} showGray={showGray} checkboxes={checkboxes} />;
   return checkboxes ? (
     <CheckboxField
       onChange={onChange}

diff --git a/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/LabelTag.jsx b/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/LabelTag.jsx
@@ -1,13 +1,12 @@
 import React from 'react';
 import ValueField from './ValueField';
 
-function LabelTag({ label, color, highlighted, dense, showGray, checkboxes, cell, valueTextAlign }) {
+function LabelTag({ label, color, dense, showGray, checkboxes, cell, valueTextAlign }) {
   if (typeof label === 'string') {
     return (
       <ValueField
         label={label}
         color={color}
-        highlighted={highlighted}
         dense={dense}
         showGray={showGray}
         checkboxes={checkboxes}

diff --git a/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/ListBoxRoot.jsx b/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/ListBoxRoot.jsx
@@ -86,15 +86,10 @@ const RowColRoot = styled('div', {
     fontSize: 12,
   },
 
-  // Highlight is added to labelText spans, which are created as siblings to original labelText,
+  // Highlight is added to labelText spans, which are created as children to original labelText,
   // when a search string is matched.
   [`& .${classes.highlighted}`]: {
-    overflow: 'visible',
-    width: '100%',
-    '& > span': {
-      width: '100%',
-      backgroundColor: '#FFC72A',
-    },
+    backgroundColor: '#FFC72A',
   },
 
   // Checkbox and label container.
@@ -118,14 +113,6 @@ const RowColRoot = styled('div', {
     },
   },
 
-  [`& .${classes.checkboxLabel} .${classes.labelText}`]: {
-    width: 'auto',
-  },
-
-  [`& .${classes.checkboxLabel} .${classes.labelText}.${classes.highlighted}`]: {
-    width: 'auto',
-  },
-
   // The icons container holding tick and lock, shown inside fields.
   [`& .${classes.icon}`]: {
     display: 'flex',

diff --git a/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/ValueField.jsx b/apis/nucleus/src/components/listbox/components/ListBoxRowColumn/components/ValueField.jsx
@@ -4,23 +4,21 @@ import { joinClassNames } from '../helpers/operations';
 import classes from '../helpers/classes';
 import { excludedOrAlternative } from '../helpers/cell-states';
 
-function ValueField({ label, color, highlighted = false, dense, showGray = true, checkboxes, cell, valueTextAlign }) {
+function ValueField({ label, color, dense, showGray = true, checkboxes, cell, valueTextAlign }) {
   return (
     <Typography
       component="span"
       variant="body2"
       className={joinClassNames([
         classes.labelText,
-        highlighted && classes.highlighted,
         dense && classes.labelDense,
         showGray && excludedOrAlternative({ cell, checkboxes }) && classes.excludedTextWithCheckbox,
       ])}
       color={color}
       align={valueTextAlign}
+      dir="auto"
     >
-      <span dir="auto" style={{ whiteSpace: 'pre' }}>
-        {label}
-      </span>
+      <span style={{ whiteSpace: 'pre' }}>{label}</span>
     </Typography>
   );
 }

diff --git a/apis/nucleus/src/utils/rtl-util.js b/apis/nucleus/src/utils/rtl-util.js