feat: search in collapsed nodes #23

Raubzeug · 2025-11-10T15:34:39Z

No description provided.

gravity-ui-bot · 2025-11-10T15:36:05Z

gravity-ui-bot · 2025-11-10T15:39:56Z

ma-efremoff · 2025-11-12T14:32:49Z

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

-
-    await page.getByTestId('qa:structuredyson:search').locator('input').fill('Attr');
+    // Wait for search to complete
+    await page.waitForTimeout(300);


Isn't it better to wait for some search results like a highlighted text or N matches?

ma-efremoff · 2025-11-12T14:33:08Z

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

    await page.getByTestId('qa:structuredyson:search').locator('input').fill('attr');

+    // Wait for search to complete
+    await page.waitForTimeout(300);


ma-efremoff · 2025-11-12T14:33:28Z

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

+    await page.getByTestId('qa:structuredyson:search:next').click();
+
+    // Wait for expansion and navigation
+    await page.waitForTimeout(300);


ma-efremoff · 2025-11-13T14:54:35Z

src/StructuredYson/StructuredYson.tsx

+                const count = allMatchPaths.filter((matchPath) => {
+                    // Match if path is exactly the item path (match at the node itself)
+                    // or starts with prefix (match inside the node)
+                    return matchPath === item.path || matchPath.startsWith(prefix);


There is no need to compare the strings twice:

const sameStart = matchPath.startsWith(item.path); if (item.path.length === matchPath.length ) { // exact match return true; } if (matchPath[item.path.length] === '/') { // match inside the node return true; } return false;

ma-efremoff · 2025-11-13T15:23:05Z

src/StructuredYson/StructuredYson.tsx

+            if (collapsedState[checkPath]) {
+                return checkPath;
+            }
+        }


It is better to avoid unnecessary split/join calls:

let nextSlash: number = -1; while(true) { nextSlash = matchPath.nextIndex('/', nextSlash + 1) if (nextSlash === -1) { break; } const checkPath = matchPath.slice(0, nextSlash); if (collapsedState[checkPath]) { return checkPath; } }

ma-efremoff · 2025-11-13T15:31:58Z

src/StructuredYson/StructuredYson.tsx

-        }
+        // Calculate next index in total matches
+        let nextTotalIndex = matchIndex + diff;
+        nextTotalIndex = ((nextTotalIndex % totalMatches) + totalMatches) % totalMatches;


Isn't it the same for:

const nextTotalIndex = (matchIndex + diff) % totalMatches;

?

Cannot simplify ((nextTotalIndex % totalMatches) + totalMatches) % totalMatches because:

onPrevMatch() calls onNextMatch(null, -1) for backward navigation

This makes nextTotalIndex = matchIndex + (-1) potentially negative (e.g., when matchIndex = 0)

JavaScript's % operator returns negative results: (-1) % 5 = -1 (incorrect index)

const nextTotalIndex = (totalMatches + matchIndex + diff) % totalMatches;

ma-efremoff · 2025-11-13T15:49:00Z

src/StructuredYson/StructuredYson.tsx

-        this.searchRef.current?.focus();
+        // Target is hidden - expand the collapsed parent
+        const newCollapsedState = {...collapsedState};
+        delete newCollapsedState[collapsedParent];


As I understand it expands only one level for each call of this.onNextMatch. Can we expand all parents here before call retry this.onNextMatch(...)?

ma-efremoff · 2025-11-13T16:04:13Z

src/utils/flattenUnipika.ts

+        paths.push(...collectMapMatches(value.$value, filter, settings, isJson, valuePath));
+    } else if (value.$type === 'list' && value.$value) {
+        paths.push(...collectListMatches(value.$value, filter, settings, isJson, valuePath));
+    }


It looks like it is better to addelse {...} block here for primitive types:

} else { const valueMatch = rowSearchInfo(value, filter, settings); if (vauleMatch && currentPath) { paths.push(currentPath); } }

It looks like you have forgotten about it, haven't you?

sorry! fixed!

We don't need checkPrimitiveMatch function anymore.

Additionally it does unnecessary check

ma-efremoff · 2025-11-13T16:09:06Z

src/utils/flattenUnipika.ts

+    paths.push(...collectAttributeMatches(value, filter, settings, isJson, currentPath));
+
+    // Get value path with JSON $ prefix if needed
+    const valuePath = getJsonValuePath(value, isJson, currentPath);


We need to calculate the value only for processing structures.

ma-efremoff · 2025-11-13T16:14:54Z

src/utils/flattenUnipika.ts

+/**
+ * Collect all paths that contain search matches (including in collapsed nodes)
+ */
+function collectAllMatchPaths(


It is more efficient to pass paths as an additional parameter to avoid unnecessary memory allocations for internal arrays, like:

function collectAllMatchPaths( dstPaths: Array<string>, ... ) { ... return dstPaths; }

ma-efremoff · 2025-11-14T13:13:15Z

src/StructuredYson/StructuredYson.tsx

-                this.tableRef.current?.scrollToIndex(matchedRows[visibleMatchCount]);
-                this.searchRef.current?.focus();
+            nextSlash++;
+        }


Is it possible to avoid copy-paste from findCollapsedParent here, like:

const expandCollapsedPath = (path: string) { if (effectiveCollapsedState === collapsedState) { effectiveCollapsedState = {...collapsedState}; } delete effectiveCollapsedState[collapsedParent]; } let collapsedParent: string | undefined; while(collapsedParent = this.findCollapsedParent(..., effectiveCollapsedState)) { expandPath(collapsedParent); }

?

ma-efremoff · 2025-11-14T13:18:02Z

src/StructuredYson/StructuredYson.tsx

+        }
+        // Check the full path as well
+        if (collapsedState[targetMatchPath]) {
+            if (!hasCollapsedParents) {


if (collapsedPath) { expandPath(targetMatchPath) }

refactored!

ma-efremoff · 2025-11-14T13:19:47Z

src/StructuredYson/StructuredYson.tsx

+        }
+
+        // If we expanded any parents, recalculate state and retry
+        if (hasCollapsedParents) {


if (collapsedState !== effectiveCollapsedState) { ...

ma-efremoff · 2025-11-14T13:27:27Z

src/StructuredYson/StructuredYson.tsx

+            this.setState({matchIndex: nextTotalIndex});
+            this.tableRef.current?.scrollToIndex(matchedRows[visibleMatchCount]);
+            this.searchRef.current?.focus();
+        }


Line ranges 310-326 and 334-346 looks like a copy paste. Isn't it better to extract it to a local helper, like:

const navigateTo = ({matchIndex, matchedRows, collapsedState}) => { ... }

Is there any blockers for such approach?

no blockers!

ma-efremoff · 2025-11-14T13:30:43Z

src/utils/flattenUnipika.ts

+        paths.push(...collectMapMatches(value.$value, filter, settings, isJson, valuePath));
+    } else if (value.$type === 'list' && value.$value) {
+        paths.push(...collectListMatches(value.$value, filter, settings, isJson, valuePath));
+    }


It looks like you have forgotten about it, haven't you?

ma-efremoff · 2025-11-14T14:17:40Z

src/StructuredYson/StructuredYson.tsx

+                }
+            });
+            return;
        }


~~Shouldn't we add else here for next call?~~
I see there is return in the end of if block

Raubzeug force-pushed the search-folded branch from 036f12f to 14bacf0 Compare November 11, 2025 12:56

Raubzeug marked this pull request as ready for review November 11, 2025 12:57

Raubzeug requested a review from ma-efremoff as a code owner November 11, 2025 12:57

Raubzeug added 3 commits November 11, 2025 16:18

feat: search in collapsed nodes

7f69426

fix: tests

4eb9754

fix: tests

637478c

Raubzeug force-pushed the search-folded branch from 14bacf0 to 637478c Compare November 11, 2025 13:26

Raubzeug added 3 commits November 11, 2025 16:32

fix: test

89fe957

fix: remove property

2f7b416

fix: units

64bc017

ma-efremoff reviewed Nov 13, 2025

View reviewed changes

fix: review

8fa0e3c

ma-efremoff reviewed Nov 14, 2025

View reviewed changes

Raubzeug added 3 commits November 14, 2025 16:48

fix: review

6db29c3

fix: review

83c5385

fix: review

cc79fd2

Raubzeug requested a review from ma-efremoff November 14, 2025 14:12

ma-efremoff reviewed Nov 14, 2025

View reviewed changes

ma-efremoff approved these changes Nov 14, 2025

View reviewed changes

Raubzeug merged commit ffaccbe into main Nov 14, 2025
3 checks passed

Raubzeug deleted the search-folded branch November 14, 2025 14:31

gravity-ui-bot mentioned this pull request Nov 11, 2025

chore(main): release 0.3.0 #24

Merged

feat: search in collapsed nodes #23

feat: search in collapsed nodes #23

Uh oh!

Conversation

Raubzeug commented Nov 10, 2025

Uh oh!

gravity-ui-bot commented Nov 10, 2025

Uh oh!

gravity-ui-bot commented Nov 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ma-efremoff Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ma-efremoff Nov 14, 2025 •

edited

Loading