Skip to content
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

feat: Add "latest" and "related" search. #2055

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions assets/css/search-bar.css
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
color: var(--searchAccentMain);
position: relative;
height: 46px;
padding: 8px 35px 8px 43px;
padding: 8px 135px 8px 43px;
width: 100%;
transition: var(--transition-all);
}
Expand Down Expand Up @@ -80,7 +80,8 @@
background-color: transparent;
border: none;
cursor: pointer;
right: 11px;
/* FIXME: I know this is wrong. Not sure what to do though */
right: 100px;
margin: 0;
opacity: 0.5;
padding: 5px 1px 5px 0;
Expand All @@ -95,6 +96,31 @@
opacity: 0.7;
}

.top-search .search-bar .search-type {
background-color: transparent;
border-top: 1px solid transparent;
border-bottom: 1px solid transparent;
border-right: 1px solid transparent;
border-left: 1px solid var(--searchBarBorder);
border-top-right-radius: var(--borderRadius-base);
border-bottom-right-radius: var(--borderRadius-base);
color: var(--searchAccentMain);
position: absolute;
top: calc(50% - 23px);
right: 0;
height: 46px;
padding: 8px 8px 8px 16px;
transition: var(--transition-all);
z-index: 99;
}

.top-search .search-bar .search-type:focus {
border: 1px solid var(--searchBarFocusColor);
border-top-right-radius: calc(var(--borderRadius-base) - 1px);
border-bottom-right-radius: calc(var(--borderRadius-base) - 1px);
box-shadow: 0px 4px 20px 0px var(--searchBarBorderColor) inset;
}

.top-search .search-settings button.icon-settings {
display: flex;
align-items: center;
Expand Down
27 changes: 2 additions & 25 deletions assets/js/autocomplete/suggestions.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getSidebarNodes } from '../globals'
import { escapeRegexModifiers, escapeHtmlEntities, isBlank } from '../helpers'
import { isBlank } from '../helpers'
import { highlightMatches } from '../highlighter'

/**
* @typedef Suggestion
Expand Down Expand Up @@ -285,27 +286,3 @@ function startsWith (text, subtext) {
function tokenize (query) {
return query.trim().split(/\s+/)
}

/**
* Returns an HTML string highlighting the individual tokens from the query string.
*/
function highlightMatches (text, query) {
// Sort terms length, so that the longest are highlighted first.
const terms = tokenize(query).sort((term1, term2) => term2.length - term1.length)
return highlightTerms(text, terms)
}

function highlightTerms (text, terms) {
if (terms.length === 0) return text

const [firstTerm, ...otherTerms] = terms
const match = text.match(new RegExp(`(.*)(${escapeRegexModifiers(firstTerm)})(.*)`, 'i'))

if (match) {
const [, before, matching, after] = match
// Note: this has exponential complexity, but we expect just a few terms, so that's fine.
return highlightTerms(before, terms) + '<em>' + escapeHtmlEntities(matching) + '</em>' + highlightTerms(after, terms)
} else {
return highlightTerms(text, otherTerms)
}
}
4 changes: 4 additions & 0 deletions assets/js/globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ export function getSidebarNodes () {
export function getVersionNodes () {
return window.versionNodes || []
}

export function getSearchNodes () {
return window.searchNodes || []
}
34 changes: 34 additions & 0 deletions assets/js/highlighter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { escapeRegexModifiers, escapeHtmlEntities } from './helpers'

/**
* Returns an HTML string highlighting the individual tokens from the query string.
*/
export function highlightMatches (text, query, opts = {}) {
// Sort terms length, so that the longest are highlighted first.
if (typeof query === 'string') {
query = query.split(/\s+/)
}
const terms = query.sort((term1, term2) => term2.length - term1.length)
return highlightTerms(text, terms, opts)
}

function highlightTerms (text, terms, opts) {
if (terms.length === 0) return text

let flags = 'i'

if (opts.multiline) {
flags = 'is'
}

const [firstTerm, ...otherTerms] = terms
const match = text.match(new RegExp(`(.*)(${escapeRegexModifiers(firstTerm)})(.*)`, flags))

if (match) {
const [, before, matching, after] = match
// Note: this has exponential complexity, but we expect just a few terms, so that's fine.
return highlightTerms(before, terms, opts) + '<em>' + escapeHtmlEntities(matching) + '</em>' + highlightTerms(after, terms, opts)
} else {
return highlightTerms(text, otherTerms, opts)
}
}
92 changes: 89 additions & 3 deletions assets/js/search-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
AUTOCOMPLETE_CONTAINER_SELECTOR,
AUTOCOMPLETE_SUGGESTION_LIST_SELECTOR
} from './autocomplete/autocomplete-list'
import { isEmbedded } from './globals'
import { isEmbedded, getSearchNodes, getVersionNodes } from './globals'
import { isAppleOS, qs } from './helpers'

const SEARCH_INPUT_SELECTOR = 'form.search-bar input'
Expand All @@ -37,6 +37,12 @@ function initialize () {
focusSearchInput()
if (open) { showPreview(event.target) } else { hidePreview() }
}

if (window.navigator.onLine) {
enableRemoteSearch(null)
} else {
disableRemoteSearch(null)
}
}

/**
Expand Down Expand Up @@ -137,6 +143,66 @@ function addEventListeners () {
clearSearch()
hideAutocomplete()
})

window.addEventListener('online', enableRemoteSearch)
window.addEventListener('offline', disableRemoteSearch)
}

function enableRemoteSearch (_event) {
Array.from(document.getElementsByClassName('online-only')).forEach(element => {
element.removeAttribute('disabled')
element.removeAttribute('title')
updateSearchTypeOptions(element)
})
}

function disableRemoteSearch (_event) {
Array.from(document.getElementsByClassName('online-only')).forEach(element => {
element.setAttribute('disabled', '')
element.setAttribute('title', 'Local searching only - browser is offline')
updateSearchTypeOptions(element)
})
}

function updateSearchTypeOptions (element) {
Array.from(element.getElementsByTagName('option')).forEach(option => {
if (option.value === 'related') {
if (shouldEnableRelatedSearch()) {
option.removeAttribute('disabled')
option.removeAttribute('title')
} else {
option.setAttribute('disabled', '')
option.setAttribute('title', 'No known related packages to search')
}
}
if (option.value === 'latest') {
if (shouldEnableLatestSearch()) {
option.removeAttribute('disabled')
option.removeAttribute('title')
} else {
option.setAttribute('disabled', '')
option.setAttribute('title', 'Already browsing latest version')
}
}
})
}

function shouldEnableRelatedSearch () {
return getSearchNodes().length > 1
}

function shouldEnableLatestSearch () {
const versionNodes = getVersionNodes()

if (versionNodes.length > 0) {
const latest = versionNodes[0]
const searchNodes = getSearchNodes()
const match = searchNodes.some(node => `v${node.version}` === latest.version)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to check if latest or not. It is easier if the user doesn't have to think about it. Regardless if it we are in the latest package or not, we should allow latest to be chosen (unless there is no searchNodes). So basically, we will have this:

  1. If searchNodes.length > 1, the related option is enabled, otherwise it is disabled
  2. If searchNodes.length > 0, the latest option is enabled, otherwise it is disabled
  3. Otherwise it is only current/local

If online, the default should be related > latest > current. If offline, it is the current one.

In other words, this should return return versionNodes.length > 0.


return !match
}

return false
}

function handleAutocompleteFormSubmission (event) {
Expand All @@ -155,8 +221,18 @@ function handleAutocompleteFormSubmission (event) {
anchor.setAttribute('href', autocompleteSuggestion.link)
} else {
const meta = document.querySelector('meta[name="exdoc:full-text-search-url"]')
const url = meta ? meta.getAttribute('content') : 'search.html?q='
anchor.setAttribute('href', `${url}${encodeURIComponent(searchInput.value)}`)
const url = meta ? meta.getAttribute('content') : 'search.html'

const params = new URLSearchParams()
params.set('q', searchInput.value)

const searchType = getSearchType()

if (searchType !== 'local') {
params.set('type', searchType)
}

anchor.setAttribute('href', `${url}?${params.toString()}`)
}

anchor.click()
Expand All @@ -167,6 +243,16 @@ function handleAutocompleteFormSubmission (event) {
}
}

function getSearchType () {
const searchTypes = Array.from(document.getElementsByClassName('search-type'))

if (searchTypes.length > 0) {
return searchTypes[0].value
}

return 'local'
}

function clearSearch () {
const input = qs(SEARCH_INPUT_SELECTOR)
input.value = ''
Expand Down
83 changes: 75 additions & 8 deletions assets/js/search-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import lunr from 'lunr'
import { qs, escapeHtmlEntities, isBlank, getQueryParamByName, getProjectNameAndVersion } from './helpers'
import { setSearchInputValue } from './search-bar'
import searchResultsTemplate from './handlebars/templates/search-results.handlebars'
import { getSearchNodes } from './globals'
import { highlightMatches } from './highlighter'

const EXCERPT_RADIUS = 80
const SEARCH_CONTAINER_SELECTOR = '#search'
Expand All @@ -26,30 +28,95 @@ function initialize () {
const pathname = window.location.pathname
if (pathname.endsWith('/search.html') || pathname.endsWith('/search')) {
const query = getQueryParamByName('q')
search(query)
const queryType = getQueryParamByName('type')
search(query, queryType)
}
}

async function search (value) {
async function search (value, queryType) {
if (isBlank(value)) {
renderResults({ value })
} else {
setSearchInputValue(value)

const index = await getIndex()

try {
// We cannot match on atoms :foo because that would be considered
// a filter. So we escape all colons not preceded by a word.
const fixedValue = value.replaceAll(/(\B|\\):/g, '\\:')
const results = searchResultsToDecoratedSearchItems(index.search(fixedValue))
let results = []
const searchNodes = getSearchNodes()

if (['related', 'latest'].includes(queryType) && searchNodes.length > 0) {
setSearchType(queryType)

results = await remoteSearch(value, queryType, searchNodes)
Copy link
Contributor

@ruslandoga ruslandoga Jan 22, 2025

Choose a reason for hiding this comment

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

Just a couple nitpicks :)

Can we have a race condition here? When the previous request returns after the current request and updates the items to stale results. I think it's possible with multiple HTTP/1.1 connections, but not sure about multiple streams on the same HTTP/2 connection, are they guaranteed to be ordered? Or maybe JS runtime resolves it in some way?

Also, do we need to debounce on remote search or check for response.ok and results.length > 0?

For some reason I decided to do these things in ruslandoga#1 but I don't remember if I actually had these problems or was just playing it safe ...

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I'm sure you're right. As you can probably tell it's been almost a decade since I wrote any JavaScript so I'm still getting the hang of the new idioms.

Copy link
Author

Choose a reason for hiding this comment

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

So looking at the code more carefully, it appears that the search function is only called on page load, so should only be run once in the page's lifecycle.

Copy link
Author

Choose a reason for hiding this comment

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

Additionally, the search result handlebars template takes care of whether any results were actually returned.

} else {
results = await localSearch(value)
}

renderResults({ value, results })
} catch (error) {
renderResults({ value, errorMessage: error.message })
}
}
}

async function localSearch (value) {
const index = await getIndex()

// We cannot match on atoms :foo because that would be considered
// a filter. So we escape all colons not preceded by a word.
Copy link
Member

Choose a reason for hiding this comment

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

Is this considered a filter by typesense?

const fixedValue = value.replaceAll(/(\B|\\):/g, '\\:')
return searchResultsToDecoratedSearchItems(index.search(fixedValue))
}

async function remoteSearch (value, queryType, searchNodes) {
let filterNodes = searchNodes

if (queryType === 'latest') {
filterNodes = searchNodes.slice(0, 1)
}

const filters = filterNodes.map(node => `${node.name}-${node.version}`).join(',')

const params = new URLSearchParams()
params.set('q', value)
params.set('query_by', 'title,doc')
params.set('filter_by', `package:=[${filters}]`)

const response = await fetch(`https://search.hexdocs.pm/?${params.toString()}`)
const payload = await response.json()

if (Array.isArray(payload.hits)) {
return payload.hits.map(result => {
const [packageName, packageVersion] = result.document.package.split('-')

const doc = highlightMatches(result.document.doc, value, { multiline: true })
const excerpts = [doc]
const metadata = {}
const ref = `https://hexdocs.pm/${packageName}/${packageVersion}/${result.document.ref}`
const title = result.document.title
const type = result.document.type

return {
doc,
excerpts,
metadata,
ref,
title,
type
}
})
} else {
return []
}
}

function setSearchType (value) {
const searchTypes = Array.from(document.getElementsByClassName('search-type'))

searchTypes.forEach(element => {
element.value = value
})
}

function renderResults ({ value, results, errorMessage }) {
const searchContainer = qs(SEARCH_CONTAINER_SELECTOR)
const resultsHtml = searchResultsTemplate({ value, results, errorMessage })
Expand Down
Loading