Repository navigation
fix(web): seek the first transcript search result - #2884
Conversation
|
@greptileai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| describe("transcript search navigation", () => { | ||
| it("seeks to the first match before advancing through later matches", async () => { | ||
| const input = await enterSearch("match"); | ||
|
|
||
| await pressEnter(input); | ||
| expect(video.currentTime).toBe(1); | ||
| expect(container.textContent).toContain("1/2"); | ||
|
|
||
| await pressEnter(input); | ||
| expect(video.currentTime).toBe(3); | ||
| expect(container.textContent).toContain("2/2"); | ||
|
|
||
| await pressEnter(input); | ||
| expect(video.currentTime).toBe(1); | ||
| expect(container.textContent).toContain("1/2"); |
There was a problem hiding this comment.
The new test only presses Enter on one unchanged query. It does not cover starting with Previous or changing the query after selecting a match, both of which depend on the new -1 state. Those paths could break while this test still passes. Add checks that fresh Previous seeks the last match and that the first Next after changing the query seeks the new first match.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/__tests__/unit/transcript-sidebar-search.test.ts
Line: 159-173
Comment:
**Search paths lack test coverage**
The new test only presses Enter on one unchanged query. It does not cover starting with Previous or changing the query after selecting a match, both of which depend on the new `-1` state. Those paths could break while this test still passes. Add checks that fresh Previous seeks the last match and that the first Next after changing the query seeks the new first match.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Added both, Previous on a fresh search seeks the last match and the first Next after changing the query seeks the new first match. Both pass with the current logic so no component change was needed (4fe95e0).
|
@greptileai review |
in the transcript editor, a new search showed 1/N but the first enter or next jumped straight to the second match, because the index started on the first result and then moved before seeking. you had to cycle all the way round to reach the first one.
now a fresh search starts with nothing selected, so next goes to the first match and previous goes to the last. the counter still reads 1/N before you move. the test renders the sidebar with a real video element and checks where it seeks.
The PR appears safe to merge, with no outstanding findings.Summary
The PR fixes the first transcript search jump and adds tests for search navigation.
Reviews (2) · Last reviewed commit: "test(web): cover transcript search navig..." · Reviewed by Greptile