Skip to content

feat: Scroll to search block location #509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

weilirs
Copy link
Collaborator

@weilirs weilirs commented Mar 24, 2025

 

Screen.Recording.2025-03-24.at.11.41.05.mov

@weilirs weilirs requested a review from milaiwi March 24, 2025 15:44
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR implements a scroll-to-content feature for search results and similar entries, allowing users to navigate directly to relevant content when opening files.

  • Added sophisticated content matching in FileContext.tsx with multiple fallback strategies (exact match, partial match, word-by-word search) to handle various text scenarios
  • Implemented text extraction logic in DBResultPreview.tsx and SimilarEntriesComponent.tsx to identify meaningful content snippets for scrolling
  • Added contentToScrollTo parameter propagation through context providers and components to support the scroll feature
  • Potential timing issue in FileContext.tsx with 800ms setTimeout that could be unreliable for very large documents
  • Security consideration: Content extraction uses removeMd for sanitization, but should validate input lengths and content types

💡 (5/5) You can turn off certain types of comments like style here!

6 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

const firstFewLines = lines.slice(0, 5)
const titleLine = firstFewLines.find((line) => {
const trimmed = line.trim()
return trimmed && trimmed.length > 3 && trimmed.length < 100
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: length > 3 may miss valid single-word titles


onSelect(path, textToFind)
}

if (similarEntries.length > 0) {
content = (
<div className="size-full">
{similarEntries
.filter((dbResult) => dbResult)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: filter(dbResult => dbResult) won't catch empty content - should be more specific

Comment on lines +105 to 107
onSelect={(path, content) => {
openTabContent(path, undefined, false, content)
posthog.capture('open_file_from_related_notes')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The undefined parameter passed to openTabContent could be removed since it's not being used meaningfully

Comment on lines +13 to 17
const getFileName = (path: string) => {
if (!path) return null
const parts = path.split('/')
return parts[parts.length - 1]
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: getFileName now returns null for invalid paths but this isn't handled in the UI rendering - could cause runtime errors

Suggested change
const getFileName = (path: string) => {
if (!path) return null
const parts = path.split('/')
return parts[parts.length - 1]
}
{fileName ? <span className="text-xs text-gray-400">{fileName} </span> : null} | Similarity:{' '}

Comment on lines +189 to +190
// @ts-ignore - HTMLElement does have scrollIntoView
bestMatchElement.scrollIntoView({ behavior: 'smooth', block: 'center' })
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Remove @ts-ignore and properly type HTMLElement. scrollIntoView is a standard method that shouldn't need ignoring

Suggested change
// @ts-ignore - HTMLElement does have scrollIntoView
bestMatchElement.scrollIntoView({ behavior: 'smooth', block: 'center' })
bestMatchElement.scrollIntoView({ behavior: 'smooth', block: 'center' })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant