Skip to content

Commit 8fa0e3c

Browse files
RaubzeugRaubzeug
committed
fix: review
1 parent 64bc017 commit 8fa0e3c

File tree

3 files changed

+155
-53
lines changed

3 files changed

+155
-53
lines changed

‎src/ReactUnipika/__tests__/ReactUnipika.visual.test.tsx‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,26 @@ test('ReactUnipika: search in collapsed - collapsed tree with search', async ({
105105
// Enter search term
106106
await page.getByTestId('qa:structuredyson:search').locator('input').fill('attr');
107107

108-
// Wait for search to complete
109-
await page.waitForTimeout(300);
108+
// Wait for search to complete by checking match counter is updated
109+
await page.locator('.g-ru-structured-yson__match-counter').waitFor({state: 'visible'});
110+
await page.locator('.g-ru-structured-yson__match-counter:has-text("1 / 9")').waitFor();
111+
112+
// Wait for the first match to be automatically expanded and highlighted text to be visible
113+
await page
114+
.locator('.g-ru-cell__filtered_highlighted:has-text("attr")')
115+
.first()
116+
.waitFor({state: 'visible'});
117+
118+
// Verify that only one highlighted element with "attr" is visible on screen
119+
const visibleHighlightedElements = await page
120+
.locator('.g-ru-cell__filtered_highlighted:has-text("attr")')
121+
.filter({hasText: 'attr'})
122+
.count();
123+
if (visibleHighlightedElements !== 1) {
124+
throw new Error(
125+
`Expected 1 visible highlighted element with "attr", but found ${visibleHighlightedElements}`,
126+
);
127+
}
110128

111129
await expectScreenshot({component: page});
112130
});
@@ -124,14 +142,32 @@ test('ReactUnipika: search in collapsed - navigate forward', async ({
124142
// Enter search term
125143
await page.getByTestId('qa:structuredyson:search').locator('input').fill('attr');
126144

127-
// Wait for search to complete
128-
await page.waitForTimeout(300);
145+
// Wait for search to complete by checking match counter is updated
146+
await page.locator('.g-ru-structured-yson__match-counter').waitFor({state: 'visible'});
147+
await page.locator('.g-ru-structured-yson__match-counter:has-text("1 / 9")').waitFor();
129148

130149
// Navigate forward (should expand first collapsed node with match)
131150
await page.getByTestId('qa:structuredyson:search:next').click();
132151

133-
// Wait for expansion and navigation
134-
await page.waitForTimeout(300);
152+
// Wait for expansion and navigation by checking the match counter updates to show position
153+
await page.locator('.g-ru-structured-yson__match-counter:has-text("2 / 9")').waitFor();
154+
155+
// Wait for the second match to be visible
156+
await page
157+
.locator('.g-ru-cell__filtered_highlighted:has-text("attr")')
158+
.nth(1)
159+
.waitFor({state: 'visible'});
160+
161+
// Verify that exactly 2 highlighted elements with "attr" are visible on screen
162+
const visibleHighlightedElements = await page
163+
.locator('.g-ru-cell__filtered_highlighted:has-text("attr")')
164+
.filter({hasText: 'attr'})
165+
.count();
166+
if (visibleHighlightedElements !== 2) {
167+
throw new Error(
168+
`Expected 2 visible highlighted elements with "attr", but found ${visibleHighlightedElements}`,
169+
);
170+
}
135171

136172
await expectScreenshot({component: page});
137173
});

‎src/StructuredYson/StructuredYson.tsx‎

Lines changed: 84 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,21 @@ function calculateState(
8181
// Count matches that are inside or at collapsed nodes
8282
flattenResult.data.forEach((item) => {
8383
if (item.collapsed && item.path) {
84-
const prefix = item.path + '/';
84+
const itemPath = item.path;
8585
const count = allMatchPaths.filter((matchPath) => {
86-
// Match if path is exactly the item path (match at the node itself)
87-
// or starts with prefix (match inside the node)
88-
return matchPath === item.path || matchPath.startsWith(prefix);
86+
const sameStart = matchPath.startsWith(itemPath);
87+
if (!sameStart) {
88+
return false;
89+
}
90+
if (itemPath.length === matchPath.length) {
91+
// exact match
92+
return true;
93+
}
94+
if (matchPath[itemPath.length] === '/') {
95+
// match inside the node
96+
return true;
97+
}
98+
return false;
8999
}).length;
90100
if (count > 0) {
91101
item.hiddenMatches = count;
@@ -234,12 +244,17 @@ export class StructuredYson extends React.PureComponent<Props, State> {
234244
};
235245

236246
findCollapsedParent = (matchPath: string, collapsedState: CollapsedState): string | null => {
237-
const pathParts = matchPath.split('/');
238-
for (let i = 1; i <= pathParts.length; i++) {
239-
const checkPath = pathParts.slice(0, i).join('/');
247+
let nextSlash = 0;
248+
while ((nextSlash = matchPath.indexOf('/', nextSlash)) !== -1) {
249+
const checkPath = matchPath.slice(0, nextSlash);
240250
if (collapsedState[checkPath]) {
241251
return checkPath;
242252
}
253+
nextSlash++;
254+
}
255+
// Check the full path as well
256+
if (collapsedState[matchPath]) {
257+
return matchPath;
243258
}
244259
return null;
245260
};
@@ -259,36 +274,76 @@ export class StructuredYson extends React.PureComponent<Props, State> {
259274

260275
const targetMatchPath = allMatchPaths[nextTotalIndex];
261276

262-
const collapsedParent = this.findCollapsedParent(targetMatchPath, collapsedState);
277+
// Expand all collapsed parents at once
278+
let effectiveCollapsedState = collapsedState;
279+
let hasCollapsedParents = false;
263280

264-
// If target is not hidden, it's visible - navigate to it
265-
if (collapsedParent === null) {
266-
// Count how many visible matches are before our target
267-
let visibleMatchCount = 0;
268-
for (let i = 0; i < nextTotalIndex; i++) {
269-
if (this.findCollapsedParent(allMatchPaths[i], collapsedState) === null) {
270-
visibleMatchCount++;
281+
let nextSlash = 0;
282+
while ((nextSlash = targetMatchPath.indexOf('/', nextSlash)) !== -1) {
283+
const checkPath = targetMatchPath.slice(0, nextSlash);
284+
if (collapsedState[checkPath]) {
285+
if (!hasCollapsedParents) {
286+
effectiveCollapsedState = {...collapsedState};
287+
hasCollapsedParents = true;
271288
}
289+
delete effectiveCollapsedState[checkPath];
272290
}
273-
274-
// Navigate to the visible match
275-
if (visibleMatchCount < matchedRows.length) {
276-
this.setState({matchIndex: nextTotalIndex});
277-
this.tableRef.current?.scrollToIndex(matchedRows[visibleMatchCount]);
278-
this.searchRef.current?.focus();
291+
nextSlash++;
292+
}
293+
// Check the full path as well
294+
if (collapsedState[targetMatchPath]) {
295+
if (!hasCollapsedParents) {
296+
effectiveCollapsedState = {...collapsedState};
297+
hasCollapsedParents = true;
279298
}
299+
delete effectiveCollapsedState[targetMatchPath];
300+
}
301+
302+
// If we expanded any parents, recalculate state and retry
303+
if (hasCollapsedParents) {
304+
this.updateState({collapsedState: effectiveCollapsedState}, () => {
305+
// After state recalculation, find the target in the new allMatchPaths
306+
const newAllMatchPaths = this.state.allMatchPaths;
307+
const newTargetIndex = newAllMatchPaths.indexOf(targetMatchPath);
308+
if (newTargetIndex !== -1) {
309+
// Navigate to the target using its new index
310+
const newMatchedRows = this.state.matchedRows;
311+
let visibleMatchCount = 0;
312+
for (let i = 0; i < newTargetIndex; i++) {
313+
if (
314+
this.findCollapsedParent(
315+
newAllMatchPaths[i],
316+
this.state.collapsedState,
317+
) === null
318+
) {
319+
visibleMatchCount++;
320+
}
321+
}
322+
if (visibleMatchCount < newMatchedRows.length) {
323+
this.setState({matchIndex: newTargetIndex});
324+
this.tableRef.current?.scrollToIndex(newMatchedRows[visibleMatchCount]);
325+
this.searchRef.current?.focus();
326+
}
327+
}
328+
});
280329
return;
281330
}
282331

283-
// Target is hidden - expand the collapsed parent
284-
const newCollapsedState = {...collapsedState};
285-
delete newCollapsedState[collapsedParent];
332+
// Target is visible - navigate to it
333+
// Count how many visible matches are before our target
334+
let visibleMatchCount = 0;
335+
for (let i = 0; i < nextTotalIndex; i++) {
336+
if (this.findCollapsedParent(allMatchPaths[i], effectiveCollapsedState) === null) {
337+
visibleMatchCount++;
338+
}
339+
}
286340

287-
// Recalculate state with new collapsed state
288-
this.updateState({collapsedState: newCollapsedState, matchIndex: nextTotalIndex}, () => {
289-
// Retry navigation to the same target
290-
this.onNextMatch(null, 0);
291-
});
341+
// Navigate to the visible match
342+
if (visibleMatchCount < matchedRows.length) {
343+
this.setState({matchIndex: nextTotalIndex});
344+
this.tableRef.current?.scrollToIndex(matchedRows[visibleMatchCount]);
345+
this.searchRef.current?.focus();
346+
}
292347
};
293348

294349
onPrevMatch = () => {

‎src/utils/flattenUnipika.ts‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ export function flattenUnipika(
8989
// Collect all match paths
9090
let allMatchPaths: Array<string> | undefined;
9191
if (options?.filter && options?.settings) {
92-
allMatchPaths = collectAllMatchPaths(
92+
allMatchPaths = [];
93+
collectAllMatchPaths(
94+
allMatchPaths,
9395
value,
9496
options.filter,
9597
options.settings,
@@ -581,8 +583,7 @@ function collectAttributeMatches(
581583
if (value.$attributes && value.$attributes.length > 0) {
582584
for (const [_key, attrValue] of value.$attributes) {
583585
const attrPath = currentPath ? `${currentPath}/@` : '@';
584-
const attrPaths = collectAllMatchPaths(attrValue, filter, settings, isJson, attrPath);
585-
paths.push(...attrPaths);
586+
collectAllMatchPaths(paths, attrValue, filter, settings, isJson, attrPath);
586587
}
587588
}
588589
return paths;
@@ -624,8 +625,7 @@ function collectMapMatches(
624625
}
625626

626627
// Recursively collect from value
627-
const childPaths = collectAllMatchPaths(childValue, filter, settings, isJson, childPath);
628-
paths.push(...childPaths);
628+
collectAllMatchPaths(paths, childValue, filter, settings, isJson, childPath);
629629
}
630630
return paths;
631631
}
@@ -643,8 +643,7 @@ function collectListMatches(
643643
const paths: Array<string> = [];
644644
for (let i = 0; i < listValue.length; i++) {
645645
const childPath = valuePath ? `${valuePath}/${i}` : String(i);
646-
const childPaths = collectAllMatchPaths(listValue[i], filter, settings, isJson, childPath);
647-
paths.push(...childPaths);
646+
collectAllMatchPaths(paths, listValue[i], filter, settings, isJson, childPath);
648647
}
649648
return paths;
650649
}
@@ -653,33 +652,45 @@ function collectListMatches(
653652
* Collect all paths that contain search matches (including in collapsed nodes)
654653
*/
655654
function collectAllMatchPaths(
655+
dstPaths: Array<string>,
656656
value: UnipikaValue,
657657
filter: string,
658658
settings: UnipikaSettings,
659659
isJson: boolean,
660660
currentPath = '',
661661
): Array<string> {
662662
if (!filter) {
663-
return [];
663+
return dstPaths;
664664
}
665665

666-
const paths: Array<string> = [];
667-
668666
// Check primitive values
669-
paths.push(...checkPrimitiveMatch(value, filter, settings, currentPath));
667+
const primitiveMatches = checkPrimitiveMatch(value, filter, settings, currentPath);
668+
if (primitiveMatches.length > 0) {
669+
dstPaths.push(...primitiveMatches);
670+
}
670671

671672
// Check attributes
672-
paths.push(...collectAttributeMatches(value, filter, settings, isJson, currentPath));
673-
674-
// Get value path with JSON $ prefix if needed
675-
const valuePath = getJsonValuePath(value, isJson, currentPath);
673+
const attrMatches = collectAttributeMatches(value, filter, settings, isJson, currentPath);
674+
if (attrMatches.length > 0) {
675+
dstPaths.push(...attrMatches);
676+
}
676677

677678
// Process nested structures
678679
if (value.$type === 'map' && value.$value) {
679-
paths.push(...collectMapMatches(value.$value, filter, settings, isJson, valuePath));
680+
// Get value path with JSON $ prefix if needed for structures
681+
const valuePath = getJsonValuePath(value, isJson, currentPath);
682+
const mapMatches = collectMapMatches(value.$value, filter, settings, isJson, valuePath);
683+
if (mapMatches.length > 0) {
684+
dstPaths.push(...mapMatches);
685+
}
680686
} else if (value.$type === 'list' && value.$value) {
681-
paths.push(...collectListMatches(value.$value, filter, settings, isJson, valuePath));
687+
// Get value path with JSON $ prefix if needed for structures
688+
const valuePath = getJsonValuePath(value, isJson, currentPath);
689+
const listMatches = collectListMatches(value.$value, filter, settings, isJson, valuePath);
690+
if (listMatches.length > 0) {
691+
dstPaths.push(...listMatches);
692+
}
682693
}
683694

684-
return paths;
695+
return dstPaths;
685696
}

0 commit comments

Comments
 (0)