Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task/WP-72: Highlight matching search terms #873

Merged
merged 12 commits into from
Oct 24, 2023
Merged
1 change: 1 addition & 0 deletions client/src/components/Jobs/Jobs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ function JobsView({
}
getRowProps={rowProps}
columnMemoProps={[version]} /* TODOv3: dropV2Jobs. */
searchTerm={query.query_string}
/>
</>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Link, useLocation } from 'react-router-dom';
import * as ROUTES from '../../../constants/routes';
import { getOutputPath } from 'utils/jobsUtil';
import './HighlightSearchTerm.scss';

const HighlightSearchTerm = ({ searchTerm, cell, id }) => {
const highlightParts = (content) => {
const parts = content.split(new RegExp(`(${searchTerm})`, 'gi'));
return parts.map((part, i) =>
part.toLowerCase() === searchTerm.toLowerCase() ? (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This look great.
Minor suggestions to help improve perf.

  1. Instantiate a regex and use it for split and comparison in line 9. Doing toLowerCase on searchTerm for each part is unnecessary. You have the regex defined for it already. Regex works better on shorter strings which is the split part.
  2. If searchTerm is not available, return immediately.

I tried to see if just string replace with regex would work instead of split and replace, I did not get one working. But, may be you can try to see if that feasible?

suggestion code:

const highlightParts = () => {
    if (!searchTerm) {
      return content;
    }
    const searchTermRegex = new RegExp(`(${searchTerm})`, 'gi');
    const parts = content.split(searchTermRegex);
    return parts.map((part, i) =>
    part.match(searchTermRegex) ? (
        <b className={styles['highlight']} key={i}>
          {part}
        </b>
      ) : (
        part
      )
    );
  };

Copy link
Contributor Author

@asimregmi asimregmi Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed logic to use match instead of toLowerCase. Tried the other suggestion to use .replace() function instead but couldn't get it to work properly. I am getting [object] [object' inside the string that needs to be highlighted. Is it because .replace() isn't good for returning the <b>...</b> and only good for string? Let me know.

<b className="highlight" key={i}>
{part}
</b>
) : (
part
)
);
};

if (id == 'Output Location') {
const outputLocation = getOutputPath(cell.row.original);

return outputLocation ? (
<Link
to={`${ROUTES.WORKBENCH}${ROUTES.DATA}/tapis/private/${outputLocation}`}
className="wb-link job__path"
>
{highlightParts(outputLocation)}
</Link>
) : null;
} else if (id == 'uuid') {
return <b className="highlight">{cell.render('Cell')}</b>;
} else if (id == 'name') {
const jobName = cell.row.values[id];

return <span>{highlightParts(jobName)}</span>;
}

return null;
};

HighlightSearchTerm.propTypes = {
searchTerm: PropTypes.string,
cell: PropTypes.object,
id: PropTypes.string,
};

HighlightSearchTerm.defaultProps = {
searchTerm: '',
outputLocation: '',
};

export default HighlightSearchTerm;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.highlight {
font-weight: bold;
}
asimregmi marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions client/src/components/_common/HighlightSearchTerm/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import HighlightSearchTerm from './HighlightSearchTerm';

export default HighlightSearchTerm;
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useTable } from 'react-table';
import PropTypes from 'prop-types';
import LoadingSpinner from '../LoadingSpinner';
import './InfiniteScrollTable.scss';
import { HighlightSearchTerm } from '_common';

const rowContentPropType = PropTypes.oneOfType([
PropTypes.string,
Expand Down Expand Up @@ -54,6 +55,7 @@ const InfiniteScrollTable = ({
noDataText,
getRowProps,
columnMemoProps,
searchTerm,
}) => {
const columns = React.useMemo(() => tableColumns, columnMemoProps);
const data = React.useMemo(() => tableData, [tableData]);
Expand Down Expand Up @@ -101,7 +103,18 @@ const InfiniteScrollTable = ({
className: cell.column.className,
})}
>
{cell.render('Cell')}
{searchTerm !== '' &&
(cell.column.id === 'name' ||
cell.column.id === 'Output Location' ||
cell.column.id === 'uuid') ? (
<HighlightSearchTerm
searchTerm={searchTerm}
cell={cell}
id={cell.column.id}
/>
) : (
cell.render('Cell')
)}
</td>
);
})}
Expand All @@ -128,12 +141,15 @@ InfiniteScrollTable.propTypes = {
noDataText: rowContentPropType,
getRowProps: PropTypes.func,
columnMemoProps: PropTypes.arrayOf(PropTypes.any),
searchTerm: PropTypes.string,
cell: PropTypes.object,
};
InfiniteScrollTable.defaultProps = {
onInfiniteScroll: (offset) => {},
isLoading: false,
className: '',
noDataText: '',
searchTerm: '',
getRowProps: (row) => {},
columnMemoProps: [],
};
Expand Down
1 change: 1 addition & 0 deletions client/src/components/_common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export { default as Icon } from './Icon';
export { default as Message } from './Message';
export { default as InlineMessage } from './InlineMessage';
export { default as SectionMessage } from './SectionMessage';
export { default as HighlightSearchTerm } from './HighlightSearchTerm';
export { default as Sidebar } from './Sidebar';
export { default as DescriptionList } from './DescriptionList';
export { default as DropdownSelector } from './DropdownSelector';
Expand Down