Skip to content

Commit

Permalink
refactor(Listbox): Make search algorithm account for multiple ranges (#…
Browse files Browse the repository at this point in the history
…766)

* fix: highlighting

* refactor: fix algorithm and white-space on search

* fix: render highlights without wrap

* Update apis/nucleus/src/components/listbox/listbox-highlight.js

* fix: change jsdoc typedef

* fix: use theme variant instead of explicit
font size

Co-authored-by: Christian Veinfors <christian.veinfors@gmail.com>
  • Loading branch information
johanlahti and veinfors committed Feb 21, 2022
1 parent 19925fe commit 428bba5
Show file tree
Hide file tree
Showing 6 changed files with 700 additions and 430 deletions.
4 changes: 2 additions & 2 deletions apis/nucleus/src/components/listbox/ListBoxCheckbox.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const useStyles = makeStyles((theme) => ({
},
}));

export default function ListboxCheckbox({ checked, highlighted, label }) {
export default function ListboxCheckbox({ checked, label }) {
const styles = useStyles();

return (
Expand All @@ -41,7 +41,7 @@ export default function ListboxCheckbox({ checked, highlighted, label }) {
checked={checked}
tabIndex={-1}
disableRipple
className={[highlighted, styles.checkbox].join(' ').trim()}
className={styles.checkbox}
inputProps={{ 'aria-labelledby': label }}
name={label}
icon={<span className={styles.cbIcon} />}
Expand Down
142 changes: 81 additions & 61 deletions apis/nucleus/src/components/listbox/ListBoxRowColumn.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,19 @@ import { makeStyles } from '@nebula.js/ui/theme';
import Lock from '@nebula.js/ui/icons/lock';
import Tick from '@nebula.js/ui/icons/tick';
import ListBoxCheckbox from './ListBoxCheckbox';
import getSegmentsFromRanges from './listbox-highlight';

const ellipsis = {
width: '100%',
overflow: 'hidden',
textOverflow: 'ellipsis',
};

const useStyles = makeStyles((theme) => ({
row: {
flexWrap: 'nowrap',
borderBottom: `1px solid ${theme.palette.divider}`,
color: theme.palette.text.primary,
'&:focus': {
boxShadow: `inset 0 0 0 2px ${theme.palette.custom.focusOutline}`,
outline: 'none',
Expand All @@ -20,36 +28,61 @@ const useStyles = makeStyles((theme) => ({
column: {
flexWrap: 'nowrap',
borderRight: `1px solid ${theme.palette.divider}`,
color: theme.palette.text.primary,
'&:focus': {
boxShadow: `inset 0 0 0 2px ${theme.palette.custom.focusOutline}`,
outline: 'none',
},
},

// The interior wrapper for all field content.
cell: {
overflow: 'hidden',
textOverflow: 'ellipsis',
display: 'flex',
alignItems: 'center',
minWidth: 0,
flexGrow: 1,
'& span': {
whiteSpace: 'nowrap',
fontSize: '12px',
lineHeight: '16px',
userSelect: 'none',
// Note that this padding is overridden when using checkboxes.
paddingLeft: '12px',
paddingRight: '12px',
},

// The leaf node, containing the label text.
labelText: {
flexBasis: 'max-content',
lineHeight: '16px',
userSelect: 'none',
whiteSpace: 'pre', // to keep white-space on highlight
...ellipsis,
},

// Highlight is added to labelText spans, which are created as siblings to original labelText,
// when a search string is matched.
highlighted: {
overflow: 'visible',
width: '100%',
'& > span': {
width: '100%',
backgroundColor: '#FFC72A',
},
},
valueLabel: {
paddingLeft: '6px',
paddingRight: '6px',

// Checkbox and label container.
checkboxLabel: {
margin: 0,
width: '100%',
// For checkboxes the first child is the checkbox container, second is the label container.
'& > span:nth-child(2)': {
...ellipsis,
},
},

// The icons container holding tick and lock, shown inside fields.
icon: {
display: 'flex',
padding: theme.spacing(1),
},
checkboxLabel: {
margin: 0,
},

// Selection styles (S=Selected, A=Available, X=Excluded).
S: {
background: theme.palette.selected.main,
color: theme.palette.selected.mainContrastText,
Expand All @@ -66,9 +99,6 @@ const useStyles = makeStyles((theme) => ({
background: theme.palette.selected.excluded,
color: theme.palette.selected.excludedContrastText,
},
highlighted: {
backgroundColor: '#FFC72A',
},
}));

export default function RowColumn({ index, style, data, column = false }) {
Expand Down Expand Up @@ -115,59 +145,48 @@ export default function RowColumn({ index, style, data, column = false }) {
setClassArr(clazzArr);
}, [cell && cell.qState]);

const getCheckboxField = ({ lbl, highlighted, color, qElemNumber }) => {
const cb = <ListBoxCheckbox label={lbl} highlighted={highlighted} checked={isSelected} />;
const getValueField = ({ lbl, ix, color, highlighted = false }) => (
<Typography
component="span"
variant="body2"
key={ix}
className={[classes.labelText, !!highlighted && classes.highlighted]
.filter((c) => !!c)
.join(' ')
.trim()}
color={color}
>
<span style={{ whiteSpace: 'pre' }}>{lbl}</span>
</Typography>
);

const getCheckboxField = ({ lbl, color, qElemNumber }) => {
const cb = <ListBoxCheckbox label={lbl} checked={isSelected} />;
const labelTag = typeof lbl === 'string' ? getValueField({ lbl, color, highlighted: false }) : lbl;
return (
<FormControlLabel
color={color}
control={cb}
className={classes.checkboxLabel}
label={<Typography>{lbl}</Typography>}
label={labelTag}
key={qElemNumber}
/>
);
};
const getValueField = ({ lbl, highlighted, ix, color }) => (
<Typography
component="span"
key={ix}
className={[classes.valueLabel, highlighted].join(' ').trim()}
noWrap
color={color}
>
{lbl}
</Typography>
);

const label = cell ? cell.qText : '';

// Handle search highlights
// Search highlights. Split up labelText span into several and add the highlighted class to matching sub-strings.
const ranges =
(cell && cell.qHighlightRanges && cell.qHighlightRanges.qRanges.sort((a, b) => a.qCharPos - b.qCharPos)) || [];

const labels = ranges.reduce((acc, curr, ix) => {
// First non highlighted segment
if (curr.qCharPos > 0 && ix === 0) {
acc.push([label.slice(0, curr.qCharPos)]);
}

// Previous non highlighted segment
const prev = ranges[ix - 1];
if (prev) {
acc.push([label.slice(prev.qCharPos + prev.qCharPos + 1, curr.qCharPos)]);
}

// Highlighted segment
acc.push([label.slice(curr.qCharPos, curr.qCharPos + curr.qCharCount), classes.highlighted]);

// Last non highlighted segment
if (ix === ranges.length - 1 && curr.qCharPos + curr.qCharCount < label.length) {
acc.push([label.slice(curr.qCharPos + curr.qCharCount)]);
}
return acc;
}, []);
const labels = getSegmentsFromRanges(label, ranges);

const getField = checkboxes ? getCheckboxField : getValueField;
const getFieldWithRanges = ({ lbls }) => {
const labelsWithRanges = lbls.map(([lbl, highlighted], ix) => getValueField({ ix, highlighted, lbl }));
return checkboxes ? getCheckboxField({ lbl: labelsWithRanges }) : labelsWithRanges;
};

const iconStyles = {
alignItems: 'center',
Expand All @@ -177,6 +196,14 @@ export default function RowColumn({ index, style, data, column = false }) {
const showLock = isSelected && isLocked;
const showTick = !checkboxes && isSelected && !isLocked;

const cellStyle = {
display: 'flex',
alignItems: 'center',
minWidth: 0,
flexGrow: 1,
padding: checkboxes ? 0 : undefined,
};

return (
<Grid
container
Expand All @@ -191,15 +218,8 @@ export default function RowColumn({ index, style, data, column = false }) {
tabIndex={0}
data-n={cell && cell.qElemNumber}
>
<Grid
item
style={{ display: 'flex', alignItems: 'center', minWidth: 0, flexGrow: 1 }}
className={classes.cell}
title={`${label}`}
>
{ranges.length === 0
? getField({ lbl: label, color: 'inherit' })
: labels.map(([lbl, highlighted], ix) => getField({ ix, highlighted, lbl }))}
<Grid item style={cellStyle} className={classes.cell} title={`${label}`}>
{ranges.length === 0 ? getField({ lbl: label, color: 'inherit' }) : getFieldWithRanges({ lbls: labels })}
</Grid>
{(showLock || showTick) && (
<Grid item className={classes.icon}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ describe('<ListBoxCheckbox />', () => {
);
});
it('should render an unchecked checkbox', async () => {
const testRenderer = await render(<ListBoxCheckbox highlighted="highlightedClass" label="just check it" />);
const testRenderer = await render(<ListBoxCheckbox label="just check it" />);
const cbs = testRenderer.root.findAllByType(Checkbox);
expect(cbs).to.have.length(1);
const [cb] = cbs;
expect(cb.props.className).to.equal('highlightedClass checkbox');
expect(cb.props.name).to.equal('just check it');
expect(cb.props.className).to.equal('checkbox');
expect(cb.props.checked).to.equal(undefined);

expect(cb.props.icon.props.className).to.equal('cbIcon');
Expand All @@ -52,7 +51,6 @@ describe('<ListBoxCheckbox />', () => {
expect(cbs).to.have.length(1);
const [cb] = cbs;
expect(cb.props.className).to.equal('checkbox');
expect(cb.props.name).to.equal('just check it');
expect(cb.props.checked).to.equal(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import getSegmentsFromRanges from '../listbox-highlight';

describe('listbox highlight', () => {
let sandbox;

// The label we want to create segments out of, where each segment
// can be either highlighted or non-highlighted.
let label;

beforeEach(() => {
label = 'ABCDEFGHIJKLMNOPQRSTUVW';
});

before(() => {
sandbox = sinon.createSandbox();
});

afterEach(() => {
sandbox.reset();
});

after(() => {
sandbox.restore();
});

it('should return expected segments for one highlighted range only', () => {
const ranges = [{ qCharPos: 1, qCharCount: 4 }];
expect(getSegmentsFromRanges(label, ranges)).to.deep.equal([
['A', false],
['BCDE', true],
['FGHIJKLMNOPQRSTUVW', false],
]);
});
it('should return expected segment for a range starting with 0', () => {
const ranges = [{ qCharPos: 0, qCharCount: 5 }];
expect(getSegmentsFromRanges(label, ranges)).to.deep.equal([
['ABCDE', true],
['FGHIJKLMNOPQRSTUVW', false],
]);
});

it('should return expected segments for two highlighted ranges', () => {
const ranges = [
{ qCharPos: 1, qCharCount: 4 },
{ qCharPos: 7, qCharCount: 10 },
];
expect(getSegmentsFromRanges(label, ranges)).to.deep.equal([
['A', false],
['BCDE', true],
['FG', false],
['HIJKLMNOPQ', true],
['RSTUVW', false],
]);
});

it('should return expected segments for many highlighted ranges', () => {
const ranges = [
{ qCharPos: 1, qCharCount: 4 },
{ qCharPos: 6, qCharCount: 3 },
{ qCharPos: 12, qCharCount: 2 },
];
expect(getSegmentsFromRanges(label, ranges)).to.deep.equal([
['A', false],
['BCDE', true],
['F', false],
['GHI', true],
['JKL', false],
['MN', true],
['OPQRSTUVW', false],
]);
});

it('should return expected segments for many highlighted ranges starting with 0', () => {
const ranges = [
{ qCharPos: 0, qCharCount: 5 },
{ qCharPos: 6, qCharCount: 3 },
{ qCharPos: 12, qCharCount: 2 },
];
expect(getSegmentsFromRanges(label, ranges)).to.deep.equal([
['ABCDE', true],
['F', false],
['GHI', true],
['JKL', false],
['MN', true],
['OPQRSTUVW', false],
]);
});

it('should return expected segments when highlighting until the end of a label', () => {
const ranges = [
{ qCharPos: 6, qCharCount: 3 },
{ qCharPos: 12, qCharCount: 11 },
];
expect(getSegmentsFromRanges(label, ranges)).to.deep.equal([
['ABCDEF', false],
['GHI', true],
['JKL', false],
['MNOPQRSTUVW', true],
]);
});
});
Loading

0 comments on commit 428bba5

Please sign in to comment.