Skip to content

Fix bug with opening sources links - #3565

Open
saengel wants to merge 1 commit into
search-sk-sddfrom
bug/sc-46218/book-bug-on-hebrew-interface
Open

Fix bug with opening sources links#3565
saengel wants to merge 1 commit into
search-sk-sddfrom
bug/sc-46218/book-bug-on-hebrew-interface

Conversation

@saengel

@saengel saengel commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes a bug where returned sources card title links wouldn't resolve.

Code Changes

Passing the ref through to the search result card

@gitvelocity-reviewer

Copy link
Copy Markdown

📊 Code Quality Score: 1/100

12 × 0.1 (Nano ESF) = 1.2, rounded to 1

Category Score Factors
🔭 Scope 3/20 2 files, single subsystem, targeted bug fix with no new endpoints or APIs
🏗️ Architecture 1/20 No architectural change; adds optional prop following existing patterns
⚙️ Implementation 3/20 Simple nullish coalescing fix; minimal logic change; PropTypes updated
⚠️ Risk 3/20 Low risk; additive optional prop; backward-compatible; easy to revert
✅ Quality 2/15 No tests added for behavioral bug fix; PropTypes properly updated; inline comment preserved
🔒 Perf / Security 0/5 No performance or security considerations relevant to this change

Was this score accurate? 👍 Yes · 👎 No

Scored by GitVelocity · How are scores calculated?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes navigation for “sources” search result cards when clicks are handled by in-app navigation (e.g., ReaderApp.handleNavigationClick / openPanel), by ensuring the click handler receives a proper text ref string instead of a URL path with query params.

Changes:

  • Pass the underlying text ref (tref) through the sources-mode search result card props.
  • Update SearchResultCard to prefer tref over href when invoking onResultClick, while keeping href as the actual anchor URL for normal navigation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
static/js/SearchResultList.jsx Adds tref: s.ref to the sources-mode card props so the card can navigate using a real ref string.
static/js/SearchResultCard.jsx Accepts tref and uses it for onResultClick(tref ?? href, ...); updates propTypes accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@stevekaplan123 stevekaplan123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm wondering why we need both tref and href. Is it possible we only need the tref?

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.

3 participants