Add search result highlighting to pipeline rows#1878
Add search result highlighting to pipeline rows#1878Mbeaulne wants to merge 1 commit into02-27-pipeline_filtersfrom
Conversation
🎩 To tophat this PR:You can add the following URL parameter to your browser to tophat this PR: |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Mbeaulne
changed the title
Mbeaulne
force-pushed
the
03-02-pipeline_filters_content_highlight
branch
3 times, most recently
from
52ced40 to
199704f
Compare
Mbeaulne
force-pushed
the
02-27-pipeline_filters
branch
from
fd1b3c9 to
caa1934
Compare
Mbeaulne
force-pushed
the
03-02-pipeline_filters_content_highlight
branch
from
199704f to
2ffd4db
Compare
|
This feels very good to use! |
| fileEntry, | ||
| getMatchMetadata(fileEntry, searchQuery, componentQuery), | ||
| ], | ||
| ); |
There was a problem hiding this comment.
As determining this filteredPipelines variable gets more complex, I'm just wondering if we should consider wrapping it in a useMemo that explicitly controls when it is calculated / processed (e.g. when the search text changes), or if the React compiler work we did will be smart enough to do that?
There was a problem hiding this comment.
react compiler should handle this
| const matchedComponentNames: string[] = []; | ||
| if (componentQuery) { | ||
| const impl = fileEntry.componentRef.spec.implementation; | ||
| if ("graph" in impl) { |
There was a problem hiding this comment.
Use existing type guard for this
| return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); | ||
| } | ||
|
|
||
| export function HighlightText({ text, query, className }: HighlightTextProps) { |
There was a problem hiding this comment.
NIT: add to react-compiler config
| isSelected?: boolean; | ||
| onSelect?: (checked: boolean) => void; | ||
| searchQuery?: string; | ||
| matchedFields?: { label: string; value: string }[]; |
There was a problem hiding this comment.
Use MatchedField instead
| searchQuery, | ||
| componentQuery, | ||
| }: { | ||
| matchedFields?: { label: string; value: string }[]; |
| const seen = new Set<string>(); | ||
| for (const task of Object.values(impl.graph.tasks)) { | ||
| const refName = task.componentRef.name ?? ""; | ||
| const specName = task.componentRef.spec?.name ?? ""; |
There was a problem hiding this comment.
NIT: I see that both condiotionals excludes seen.has(), so we can do early return:
if(seen.has(..)) {continue;}
| if (componentQuery) { | ||
| const impl = fileEntry.componentRef.spec.implementation; | ||
| if ("graph" in impl) { |
There was a problem hiding this comment.
These nested "if"s are hard to read. I advise to introduce helper (type guard) and re-use everywhere. Also use early return
| if (refName.toLowerCase().includes(q) && !seen.has(refName)) { | ||
| seen.add(refName); | ||
| matchedComponentNames.push(refName); | ||
| } | ||
| if ( | ||
| specName.toLowerCase().includes(q) && | ||
| specName !== refName && | ||
| !seen.has(specName) | ||
| ) { | ||
| seen.add(specName); | ||
| matchedComponentNames.push(specName); | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we have a case where spec !== componentRef.name - they are supposed to be the same and identical? or is it depends on how we saved it?
Otherwise we could simplify this code.
maxy-shpfy
left a comment
maxy-shpfy
left a comment
There was a problem hiding this comment.
Tophatted -works well! I would love to improve readability in usePipelineFilters
Mbeaulne
force-pushed
the
03-02-pipeline_filters_content_highlight
branch
from
2ffd4db to
4e3da05
Compare
Mbeaulne
force-pushed
the
02-27-pipeline_filters
branch
2 times, most recently
from
0d83f2e to
11e5ca9
Compare
Mbeaulne
force-pushed
the
03-02-pipeline_filters_content_highlight
branch
from
4e3da05 to
ed41519
Compare
Mbeaulne
force-pushed
the
03-02-pipeline_filters_content_highlight
branch
from
ed41519 to
dc17873
Compare
3 participants
Description
Enhanced the pipeline search functionality with text highlighting and expanded search result display. The pipeline table now highlights matching text in pipeline names, descriptions, and component names. When searching, matched descriptions are displayed below pipeline names, and matched component names are shown as badges with icons. Added a new
HighlightTextcomponent that uses regex to highlight search terms while escaping special characters for safe pattern matching.Related Issue and Pull requests
Type of Change
Checklist
Screenshots (if applicable)
Test Instructions
Additional Comments
The search results now provide better visual feedback by highlighting exactly what matched the search query, making it easier for users to understand why specific pipelines appeared in their search results.