feat: add multi-destination pathfinding with drag-and-drop reordering - #2954
feat: add multi-destination pathfinding with drag-and-drop reordering#2954julian1j wants to merge 13 commits into
Conversation
Enables chaining up to 4 nodes (1 source + 3 destinations) in pathfinding queries. Each consecutive pair fires a parallel shortest-path API call and results are merged client-side. Includes drag-and-drop reordering, node removal, and URL persistence via tertiarySearch/quaternarySearch params.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR extends pathfinding search from two fixed endpoints to up to four ordered waypoints, adding URL parameter support, multi-leg graph querying, dynamic hook state, and draggable UI updates. It also adds a ChangesMulti-node pathfinding
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/pathfindingSearch.ts`:
- Around line 48-49: The waypoint list in pathfindingSearch is compacting sparse
inputs with filter(Boolean), which can change the intended slot order for
persisted URL destinations. Update the waypoints construction in
useExploreGraph/pathfindingSearch to preserve positional gaps or explicitly
reject/normalize sparse chains before calling the path query, so primarySearch,
secondarySearch, tertiarySearch, and quaternarySearch are interpreted in their
original order.
In
`@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/usePathfindingSearch.ts`:
- Around line 59-71: The `usePathfindingSearch` effects only increase
`extraNodeCount`, so clearing `tertiarySearch` or `quaternarySearch` leaves
`totalNodeCount` stale and keeps empty waypoint rows rendered. Update the
`useEffect` blocks around `syncNodeFromParam` to also recalculate or reset
`extraNodeCount` based on the current URL params instead of only using
`Math.max`, so removed waypoints reduce the count when appropriate. Make sure
the logic handles both the tertiary and quaternary cases consistently and keeps
`setExtraNodeCount` in sync with `tertiarySearch`, `quaternarySearch`, `data2`,
and `data3`.
In
`@packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/PathfindingSearch.tsx`:
- Around line 138-151: Add a keyboard-accessible way to reorder destinations in
PathfindingSearch: the current draggable-only flow in the item wrapper and grip
icon is not reachable by keyboard users. Update the reorder UI around the
draggable row and grip control to expose focusable move controls or keyboard
event handling (for example in the same render path that uses handleDragStart,
handleDragEnter, handleDrop, and the faGripVertical grip) so users can move
items up/down without a mouse.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 73ecc1f1-2444-4b27-a40a-cee68850e4bf
📒 Files selected for processing (6)
.gitignorecmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.tsxpackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/pathfindingSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreGraph/usePathfindingSearch.tspackages/javascript/bh-shared-ui/src/hooks/useExploreParams/useExploreParams.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/PathfindingSearch.tsx
| // Build ordered list of waypoints | ||
| const waypoints = [primarySearch, secondarySearch, tertiarySearch, quaternarySearch].filter(Boolean) as string[]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don’t compact sparse waypoint params.
filter(Boolean) turns [primary, secondary, null, quaternary] into [primary, secondary, quaternary], so a sparse URL can query a different ordered path than the persisted node slots indicate. Since this PR adds URL persistence for extra destinations, disable or normalize sparse chains before querying.
Proposed guard
- const waypoints = [primarySearch, secondarySearch, tertiarySearch, quaternarySearch].filter(Boolean) as string[];
+ if (!tertiarySearch && quaternarySearch) {
+ return { enabled: false };
+ }
+
+ const waypoints = [primarySearch, secondarySearch, tertiarySearch, quaternarySearch].filter(
+ (waypoint): waypoint is string => !!waypoint
+ );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/pathfindingSearch.ts`
around lines 48 - 49, The waypoint list in pathfindingSearch is compacting
sparse inputs with filter(Boolean), which can change the intended slot order for
persisted URL destinations. Update the waypoints construction in
useExploreGraph/pathfindingSearch to preserve positional gaps or explicitly
reject/normalize sparse chains before calling the path query, so primarySearch,
secondarySearch, tertiarySearch, and quaternarySearch are interpreted in their
original order.
- Sync extraNodeCount bidirectionally with URL params so removing waypoints properly reduces the visible node count - Add keyboard accessibility to drag-and-drop reorder (arrow keys on the grip handle)
Wrap updateNode and syncNodeFromParam in useCallback to satisfy react-hooks/exhaustive-deps lint rule.
Use getByRole('textbox') to target inputs specifically, avoiding
ambiguity with the reorder grip handle aria-labels.
coderabbitai
Bot
added
enhancement
| }, []); | ||
|
|
||
| const syncNodeFromParam = useCallback( | ||
| (index: number, param: string | null, data: any) => { |
There was a problem hiding this comment.
We should consider adding a type (SearchResults | undefined) to the data param, that would help remove the any type for the node below and the need for type casting (as SearchValue | undefined ) and can safely remove that
| useEffect(() => { | ||
| if (primarySearch && sourceSearchData) { | ||
| const matchedNode = Object.values(sourceSearchData).find((node) => node.objectid === primarySearch); | ||
| syncNodeFromParam(0, primarySearch, data0); | ||
| }, [syncNodeFromParam, primarySearch, data0]); | ||
|
|
||
| if (matchedNode) { | ||
| setSourceSearchTerm(matchedNode.name); | ||
| setSourceSelectedItem(matchedNode); | ||
| } | ||
| } else { | ||
| setSourceSearchTerm(''); | ||
| setSourceSelectedItem(undefined); | ||
| } | ||
| }, [primarySearch, sourceSearchData]); | ||
| useEffect(() => { | ||
| syncNodeFromParam(1, secondarySearch, data1); | ||
| }, [syncNodeFromParam, secondarySearch, data1]); | ||
|
|
||
| useEffect(() => { | ||
| if (secondarySearch && destinationSearchData) { | ||
| const matchedNode = Object.values(destinationSearchData).find((node) => node.objectid === secondarySearch); | ||
| syncNodeFromParam(2, tertiarySearch, data2); | ||
| }, [syncNodeFromParam, tertiarySearch, data2]); | ||
|
|
||
| if (matchedNode) { | ||
| setDestinationSearchTerm(matchedNode.name); | ||
| setDestinationSelectedItem(matchedNode); | ||
| } | ||
| } else { | ||
| setDestinationSearchTerm(''); | ||
| setDestinationSelectedItem(undefined); | ||
| } | ||
| }, [secondarySearch, destinationSearchData]); | ||
| useEffect(() => { | ||
| syncNodeFromParam(3, quaternarySearch, data3); | ||
| }, [syncNodeFromParam, quaternarySearch, data3]); |
There was a problem hiding this comment.
We should maybe consider reducing from 4 useEffects to one,so everything could look something like this:
const params = [primarySearch, secondarySearch, tertiarySearch, quaternarySearch];
const searchData = [data0, data1, data2, data3];
const syncNodeFromParam = useCallback(
(index: number, param: string | null, data: SearchResults | undefined) => {
if (param && data) {
const matchedNode = Object.values(data).find((node) => node.objectid === param);
if (matchedNode) {
updateNode(index, { searchTerm: matchedNode.name, selectedItem: matchedNode });
}
} else if (!param) {
updateNode(index, emptyNode());
}
},
[updateNode]
);
const syncAllNodesFromParams = () => {
params.forEach((param, index) => syncNodeFromParam(index, param, searchData[index]));
};
// Sync URL params to node state
useEffect(() => {
syncAllNodesFromParams();
}, [syncNodeFromParam, ...params, ...searchData]);
| const term = selected?.name ?? objectId; | ||
| // Keep extraNodeCount in sync with URL params | ||
| useEffect(() => { | ||
| const count = quaternarySearch ? 2 : tertiarySearch ? 1 : 0; |
There was a problem hiding this comment.
nit: wonder if we can make this ternary a bit more readable, one suggestion could be to do something like:
const extraCount = [tertiarySearch, quaternarySearch].filter(Boolean).length;
setExtraNodeCount(extraCount);
The caveat here is that it assumes that if we have a quaternary we would have the tertiary, to get the right extra count of 2. I think that will always be true because we have a cleanup function for params, but not a fan of assumptions.
If not we could always put a comment on top of the terniary that says: "if quaternary then 2, if not and tertiary then 1 if none 0"
Also should consider changing the variable name to two words like extraCount, just to keep to descriptive readability to scan quicker.
| const updateNode = useCallback((index: number, update: Partial<PathfindingNode>) => { | ||
| setNodes((prev) => { | ||
| const next = [...prev]; | ||
| while (next.length <= index) next.push(emptyNode()); | ||
| next[index] = { ...next[index], ...update }; | ||
| return next; | ||
| }); |
There was a problem hiding this comment.
nice to have: we should consider using more descriptive two word var names here. for example instead of update use updatedNode, for prev -> previousNodes, next -> newNodes
Also may help to leave quick one line comments on top of the while loop and the line below to give a quick scannable way of reading it. i.e Pad the array with empty nodes to prepare for new ones, update the new list with new values
| setSourceSearchTerm(edit); | ||
| const handleReorderNodes = (fromIndex: number, toIndex: number) => { | ||
| const currentNodes = nodes.slice(0, totalNodeCount); | ||
| const [moved] = currentNodes.splice(fromIndex, 1); |
There was a problem hiding this comment.
nit: would consider giving moved a two word var name to make it a bit more descriptive
| setExtraNodeCount((prev) => prev + 1); | ||
| setNodes((prev) => { | ||
| const next = [...prev]; | ||
| while (next.length < 2 + extraNodeCount + 1) next.push(emptyNode()); |
There was a problem hiding this comment.
maybe we can consider making this:
const targetNodeCount = totalNodeCount + 1;
while (next.length < targetNodeCount) next.push(emptyNode());
| type PathfindingNode = { | ||
| searchTerm: string; | ||
| selectedItem: SearchValue | undefined; | ||
| }; | ||
|
|
||
| type PathfindingSearchState = { | ||
| sourceSearchTerm: string; | ||
| destinationSearchTerm: string; | ||
| sourceSelectedItem: SearchValue | undefined; | ||
| destinationSelectedItem: SearchValue | undefined; | ||
| nodes: PathfindingNode[]; | ||
| totalNodeCount: number; | ||
| maxNodes: number; | ||
| handleSourceNodeEdited: (edit: string) => void; | ||
| handleDestinationNodeEdited: (edit: string) => void; | ||
| handleSourceNodeSelected: (selected: SearchValue) => void; | ||
| handleDestinationNodeSelected: (selected: SearchValue) => void; | ||
| handleNodeEdited: (index: number) => (edit: string) => void; | ||
| handleNodeSelected: (index: number) => (selected: SearchValue) => void; | ||
| handleSwapPathfindingInputs: () => void; | ||
| handleReorderNodes: (fromIndex: number, toIndex: number) => void; | ||
| handleRemoveNode: (index: number) => void; | ||
| handleAddNode: () => void; | ||
| }; |
There was a problem hiding this comment.
We should consider moving these two types to usePathfindingSearch and export from there to use here. And that way in the case of the type PathfindingNode we don't have to declare it twice.
| type PathfindingNode = { | ||
| searchTerm: string; | ||
| selectedItem: SearchValue | undefined; | ||
| }; |
There was a problem hiding this comment.
lets add an export to this to be able to use in PathfindingSearch file, also incluse the PathFindingSearchState so it would look like this:
export type PathfindingNode = {
searchTerm: string;
selectedItem: SearchValue | undefined;
};
export type PathfindingSearchState = {
sourceSearchTerm: string;
destinationSearchTerm: string;
sourceSelectedItem: SearchValue | undefined;
destinationSelectedItem: SearchValue | undefined;
nodes: PathfindingNode[];
totalNodeCount: number;
maxNodes: number;
handleSourceNodeEdited: (edit: string) => void;
handleDestinationNodeEdited: (edit: string) => void;
handleSourceNodeSelected: (selected: SearchValue) => void;
handleDestinationNodeSelected: (selected: SearchValue) => void;
handleNodeEdited: (index: number) => (edit: string) => void;
handleNodeSelected: (index: number) => (selected: SearchValue) => void;
handleSwapPathfindingInputs: () => void;
handleReorderNodes: (fromIndex: number, toIndex: number) => void;
handleRemoveNode: (index: number) => void;
handleAddNode: () => void;
};
| index === 0 | ||
| ? !node.searchTerm | ||
| : index === 1 | ||
| ? !!(nodes[0]?.searchTerm && !node.searchTerm) | ||
| : !node.searchTerm, |
There was a problem hiding this comment.
Maybe readability we can consider abstracting this to a function something like:
const shouldAutoFocus = (index: number, node: PathfindingNode): boolean => {
if (index === 1) return !!(nodes[0]?.searchTerm && !node.searchTerm);
return !node.searchTerm;
};
const visibleNodes = nodes.slice(0, totalNodeCount).map((node, index) => ({
label: index === 0 ? 'Start Node' : 'Destination Node',
searchTerm: node.searchTerm,
selectedItem: node.selectedItem,
removable: index > 0 && totalNodeCount > 2,
autoFocus: shouldAutoFocus(index, node),
}));
| className={`relative flex items-center gap-1 rounded transition-all group ${ | ||
| dragIndex === index ? 'opacity-40' : '' | ||
| } ${dragOverIndex === index ? 'ring-2 ring-primary ring-offset-1' : ''}`}> |
There was a problem hiding this comment.
We should consider using the cn() function to keep consistent with how we do dynamic classes in other files. It would look like this:
className={cn('relative flex items-center gap-1 rounded transition-all group', {
'opacity-40': dragIndex === index,
'ring-2 ring-primary ring-offset-1': dragOverIndex === index,
})}>
- Type the syncNodeFromParam data param as SearchResults, dropping the any and the SearchValue cast - Collapse the four param-sync effects into one, guarded so each node re-syncs only when its own param or search data changes - Clarify the extraNodeCount ternary and rename count -> extraCount - Use descriptive names in updateNode and handleReorderNodes - Introduce targetNodeCount in handleAddNode - Export PathfindingNode and PathfindingSearchState from the hook and drop the duplicate declarations in PathfindingSearch - Extract shouldAutoFocus out of the nested ternary - Use cn() for the dynamic drag classNames Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LmiPHE4Hv6MspWEnpYvcpW
Merging main brought in KindInfoItems.test.tsx, whose mocked UseExploreParamsReturn predates the tertiarySearch/quaternarySearch params this branch adds, so it no longer satisfied the type.
| {totalNodeCount < maxNodes && ( | ||
| <button | ||
| onClick={handleAddNode} | ||
| className='flex items-center gap-1.5 text-xs text-neutral-500 hover:text-neutral-700 dark:hover:text-neutral-300 py-0.5 cursor-pointer' |
There was a problem hiding this comment.
Sorry missed this in previous feedback, but in dark mode this button is not visible because of color. We should add the dark:text-common-white and delete dark:hover:text-neutral-300 as it also makes it "disappear" in dark.
| {node.removable && ( | ||
| <button | ||
| onClick={() => handleRemoveNode(index)} | ||
| className='absolute right-1 top-1/2 -translate-y-1/2 p-1 text-neutral-500 hover:text-neutral-700 dark:hover:text-neutral-300 z-10' |
There was a problem hiding this comment.
Sorry missed this in previous feedback, but in dark mode this button is not visible because of color. We should add the dark:text-common-white and delete dark:hover:text-neutral-300 as it also makes it "disappear" in dark.
| handleReorderNodes(index, index + 1); | ||
| } | ||
| }} | ||
| className='cursor-grab text-neutral-400 hover:text-neutral-600 dark:hover:text-neutral-300 opacity-0 group-hover:opacity-100 focus:opacity-100 transition-opacity'> |
There was a problem hiding this comment.
Sorry missed this in previous feedback, but in dark mode this button is not visible because of color. We should add the dark:text-common-white and delete dark:hover:text-neutral-300 as it also makes it "disappear" in dark.
There was a problem hiding this comment.
Drag-to-reorder, add destination, and the remove "x" button are all visible in dark mode with the latest commit.
| }; | ||
|
|
||
| const SourceToBullseyeIcon = () => { | ||
| const SourceToBullseyeIcon = ({ destinationCount }: { destinationCount: number }) => { |
There was a problem hiding this comment.
In the previous version the circles/bullseyes are aligned with the center of the input. In this change they are align with bottom of input, we should consider looking for a solution to centering them like they were before.
With these changes:
vs before
There was a problem hiding this comment.
Bullseye icon/circles are now centered with the input.
Dark mode: the neutral scale is near-black under .dark, leaving the drag handle, remove button, and Add Destination button invisible. Use dark:text-common-white with a dark:hover:text-neutral-light-5 hover. Alignment: replace the fixed-pitch SourceToBullseyeIcon column with PathfindingNodeIcon inside each input row, so each icon stays centered on its input at any row height.
| <div | ||
| key={index} | ||
| draggable | ||
| onDragStart={handleDragStart(index)} | ||
| onDragEnter={handleDragEnter(index)} | ||
| onDragLeave={handleDragLeave(index)} | ||
| onDragOver={handleDragOver} | ||
| onDrop={handleDrop(index)} | ||
| onDragEnd={handleDragEnd} | ||
| className={cn('relative flex items-center gap-1 rounded transition-all group', { | ||
| 'opacity-40': dragIndex === index, | ||
| 'ring-2 ring-primary ring-offset-1': dragOverIndex === index, | ||
| })}> | ||
| <PathfindingNodeIcon isStartNode={index === 0} showConnector={index > 0} /> | ||
| <div | ||
| role='button' | ||
| tabIndex={0} | ||
| aria-label={`Reorder ${node.label}, position ${index + 1} of ${visibleNodes.length}`} | ||
| aria-roledescription='sortable' | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'ArrowUp' && index > 0) { | ||
| e.preventDefault(); | ||
| handleReorderNodes(index, index - 1); | ||
| } else if (e.key === 'ArrowDown' && index < visibleNodes.length - 1) { | ||
| e.preventDefault(); | ||
| handleReorderNodes(index, index + 1); | ||
| } | ||
| }} | ||
| className='cursor-grab text-neutral-400 hover:text-neutral-600 dark:text-common-white dark:hover:text-neutral-light-5 opacity-0 group-hover:opacity-100 focus:opacity-100 transition-opacity'> | ||
| <FontAwesomeIcon icon={faGripVertical} size='sm' /> | ||
| </div> | ||
| <div className='flex-grow'> | ||
| <ExploreSearchCombobox | ||
| autoFocus={node.autoFocus} | ||
| handleNodeEdited={handleNodeEdited(index)} | ||
| handleNodeSelected={handleNodeSelected(index)} | ||
| inputValue={node.searchTerm} | ||
| selectedItem={node.selectedItem || null} | ||
| labelText={node.label} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
When going over the suggested changes ( which worked well ty again for that ) noticed that when dragging it had the circle/bullseye icon and line as part of the "ghost" element being dragged. So we should consider only showing a drag of the input element itself. I have attached a video of how I thought it could look.
Screenshare.-.2026-07-29.3_23_22.PM.mp4
This had me thinking that we maybe should have UI/UX give a bit of input on this to make sure we are applying what they had in mind for list dragging and dropping in the app. Waiting to see what they think.
Here is how the block of code looks for that element on my end:
<div
key={index}
onDragEnter={handleDragEnter(index)}
onDragLeave={handleDragLeave(index)}
onDragOver={handleDragOver}
onDrop={handleDrop(index)}
className='relative flex items-center gap-1 rounded group'>
<PathfindingNodeIcon isStartNode={index === 0} showConnector={index > 0} />
<div
draggable
onDragStart={handleDragStart(index)}
onDragEnd={handleDragEnd}
className={cn('relative flex flex-grow items-center gap-1', {
'opacity-40': dragIndex === index,
})}>
<div
role='button'
tabIndex={0}
aria-label={`Reorder ${node.label}, position ${index + 1} of ${visibleNodes.length}`}
aria-roledescription='sortable'
onKeyDown={(e) => {
if (e.key === 'ArrowUp' && index > 0) {
e.preventDefault();
handleReorderNodes(index, index - 1);
} else if (e.key === 'ArrowDown' && index < visibleNodes.length - 1) {
e.preventDefault();
handleReorderNodes(index, index + 1);
}
}}
className='cursor-grab text-neutral-400 hover:text-neutral-600 dark:text-common-white dark:hover:text-neutral-light-5 opacity-0 group-hover:opacity-100 focus-visible:opacity-100 transition-opacity'>
<FontAwesomeIcon icon={faGripVertical} size='sm' />
</div>
<div
className={cn('flex-grow rounded transition-all', {
'ring-2 ring-primary ring-offset-1': dragOverIndex === index,
})}>
<ExploreSearchCombobox
autoFocus={node.autoFocus}
handleNodeEdited={handleNodeEdited(index)}
handleNodeSelected={handleNodeSelected(index)}
inputValue={node.searchTerm}
selectedItem={node.selectedItem || null}
labelText={node.label}
/>
</div>
{node.removable && (
<button
onClick={() => handleRemoveNode(index)}
className='absolute right-1 top-1/2 -translate-y-1/2 p-1 text-neutral-500 hover:text-neutral-700 dark:text-common-white dark:hover:text-neutral-light-5 z-10'
aria-label='Remove destination'
title='Remove destination'>
<FontAwesomeIcon icon={faTimes} size='sm' />
</button>
)}
</div>
</div>
There was a problem hiding this comment.
ah, yes, this was just introduced when i applied the alignment fix because the draggable element is the entire row div, and moving things around to fix alignment issues introduced this behavior. agree that it doesn't look ideal
There was a problem hiding this comment.
I'm looking at two options; I can either snapshot a smaller subset and just detach the icons from the input that we want to "ghost", or I can take the icon out of the row that was created with the alignment fix and rearrange the panel as a multi-column grid, the drag/drop ghost would just grab everything in the right column in that case.
There was a problem hiding this comment.
Check out the code block I added above, that one shows the option you mentioned about just detaching the icon to a outer wrapper. Feels like less of a lift. Also the outline on drag is only around the input in that block I added. Feels a bit off that an outline is around everything when it normally around an input/element. In any case going to get some guidance from UI/UX to confirm we are on the right track with what they want, making sure we are consistent with the visual change they are starting to implement in the app. Thank again for all your changes!
| handleReorderNodes(index, index + 1); | ||
| } | ||
| }} | ||
| className='cursor-grab text-neutral-400 hover:text-neutral-600 dark:text-common-white dark:hover:text-neutral-light-5 opacity-0 group-hover:opacity-100 focus:opacity-100 transition-opacity'> |
There was a problem hiding this comment.
There is a small but noticeable scenario where after you drag and drop, this icon stays visible until you click away. The fix for this would be to use focus-visible:opacity-100 instead of focus:opacity-100
There was a problem hiding this comment.
yup, good catch on that, just confirmed the fix locally
There was a problem hiding this comment.
I'm pushing a fix for this and a proposed solution for the drag-and-drop ghosting that captured more than it should have. I just want y'all to have a working version that addresses all of the comments above and we can refine again if needed. This is what I have being grabbed in the "ghost" of the drag and drop in the latest commit:
There was a problem hiding this comment.
Adding for Vis. Thanks again for the changes. Was going over your latest commit changes, there are three things happening still with the ghost:
- When you type in a search and see results, if you try to drag it take the results with it as well (shown in video)
- If you add a destination and focus on it and try to drag it bring the empty text results with it (shown in video)
- When you hover another row to drop (as seen in video), there is an outline that goes around everything including the circle/bullseye and it is almost on top of the circle icon. I would consider only applying outline/highlight to the input as there is not much space to the left of the bullseye to give it a bit of whitespace
Tia!
Screenshare.-.2026-08-04.4_56_26.PM.mp4
The handle used focus:opacity-100, so a mouse drag left it focused and visible until clicking elsewhere. focus-visible: keeps it shown for keyboard users without sticking after a drag. Moving the node icon into each row also pulled it and its connector line into the browser's default drag image. Snapshot a wrapper around just the row controls instead.
Every destination input exposed the same "Destination Node" accessible name, so a screen reader user tabbing the form could not tell the fields apart. Number them for assistive tech while every row keeps the same visible label: "Destination Node 2" is announced, "Destination Node" is shown. The numbered name keeps the visible text as a prefix so it still matches the screen for speech input (WCAG 2.5.3 Label in Name). ExploreSearchCombobox gained an optional ariaLabel that defaults to labelText, since it previously used one string for both the placeholder and the accessible name. The per-row remove buttons were identical for the same reason and now name their row.
Drag-and-drop behavior now snapshots the input's root element, which resolves an issue where it would take the results and empty text results with it on drag. Also moved the drop-target outline off the row and onto the input alone, so that the outline for drag/drop behavior does not include the circle/bullseye.
specter-flq
left a comment
specter-flq
left a comment
There was a problem hiding this comment.
LGTM! Thank you for the changes!
2 participants
Enables chaining up to 4 nodes (1 source + 3 destinations) in pathfinding queries. Each consecutive pair fires a parallel shortestpath API call and results are merged client-side. Includes drag-and-drop reordering, node removal, and URL persistence via tertiarySearch/quaternarySearch params.
Description
Enables chaining up to 4 nodes (1 source + 3 destinations) in pathfinding queries, similar to Google Maps multi-stop routing
Motivation and Context
Resolves BED-8075
Why is this change required? What problem does it solve?
How Has This Been Tested?
Tested locally on CE with standard example dataset.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
.claude/directory.