Fix the Algolia search results for the APIs #331
Conversation
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe Algolia autocomplete partial and related search templates/styles were updated to broaden preview types (adds "api" and "api group"), introduce build-time and runtime tag/version/product context (INITIAL_TAG, COMPONENT_NAME, INITIAL_TAGS, PRERELEASE, LATEST_ENTERPRISE), and unify tag/filter handling (applyFilterTags, mapToAlgoliaFilters). onStateChange gains prevState and seeds the preview with the first docs hit when appropriate. Endpoint-specific preview rendering (endpoint line, curl example) and view-all URLs with product/version params were added. Search Insights is lazily loaded and initialized once. Templates now render hit.product, and new global hooks allow programmatic facet toggling and external instance access. Multiple CSS files add a context dropdown UI, hide tagsPlugin, adjust layout/sizing, and simplify a detached container shadow. Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Docs Page
participant Autocomplete as Algolia Autocomplete
participant Index as Algolia Index
participant Insights as Search Insights
Note over Page: Initialization
Page->>Autocomplete: init( INITIAL_TAGS, COMPONENT_NAME, PRERELEASE, LATEST_ENTERPRISE )
Autocomplete->>Autocomplete: applyFilterTags(initialTags, INITIAL_TAG)
Page-->>Insights: lazy-load script (if not loaded)
Insights-->>Page: aa initialized (appId, apiKey)
Note over User,Autocomplete: Search interaction
User->>Autocomplete: type query / toggle filters
Autocomplete->>Autocomplete: onStateChange({state, prevState})
alt query or filters changed
Autocomplete->>Index: search(state.query, filters = mapToAlgoliaFilters(...))
Index-->>Autocomplete: hits (docs, api, api endpoint, api group)
Autocomplete->>Autocomplete: seed preview with first doc hit (if applicable)
Autocomplete->>Page: render results + preview (doc or endpoint)
Autocomplete-->>Insights: send view/click events
else no relevant change
Autocomplete->>Autocomplete: no preview update
end
sequenceDiagram
autonumber
participant FiltersSource as Source:getItems()
participant FilterMapper as mapToAlgoliaFilters()
participant UI as Render
Note over FiltersSource: Building filter set
FiltersSource->>FilterMapper: provide selected & initial tags
FilterMapper-->>FiltersSource: OR-filter array (deduped & normalized)
FiltersSource->>UI: items (with item.url || item.objectID)
alt endpoint item
UI->>UI: build endpointLine + curlExample
UI->>User: show endpoint preview pane + TOC
else doc item
UI->>User: show doc preview (title/snippet/breadcrumbs)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/partials/algolia-script.hbs (8)
2-3: Harden preview-type matching to prevent wrong target behavior for API endpointsYour logic relies on string matching across multiple places. If the index emits type labels like "Doc", "API", or "Endpoint" (vs "doc", "api endpoint"), the includes check can fail and force links to open in a new tab unnecessarily. Normalize once with a Set and use
.has().Apply this diff:
-const SUPPORTED_PREVIEW_TYPES = ['doc', 'api endpoint']; +const SUPPORTED_PREVIEW_TYPES = new Set(['doc', 'api endpoint', 'api', 'endpoint', 'api_endpoint']); ... - SUPPORTED_PREVIEW_TYPES.includes((preview.type || '').toLowerCase()) + SUPPORTED_PREVIEW_TYPES.has((preview.type || '').toLowerCase()) ... - SUPPORTED_PREVIEW_TYPES.includes((state.context.preview.type || '').toLowerCase()) + SUPPORTED_PREVIEW_TYPES.has((state.context.preview.type || '').toLowerCase()) ... - const aTag = SUPPORTED_PREVIEW_TYPES.includes((item.type || '').toLowerCase()) + const aTag = SUPPORTED_PREVIEW_TYPES.has((item.type || '').toLowerCase())Also applies to: 256-257, 300-301, 513-514
28-31: Avoid stringly-typed boolean for prerelease logic
PRERELEASEis a string; relying on'true'comparisons is brittle. Convert once to a boolean and use that forVERSION.-const VERSION = PRERELEASE === 'true' - ? `{{{page.displayVersion}}}`.trim() - : `{{{page.componentVersion.version}}}`.trim(); +const IS_PRERELEASE = PRERELEASE === 'true'; +const VERSION = IS_PRERELEASE + ? `{{{page.displayVersion}}}`.trim() + : `{{{page.componentVersion.version}}}`.trim();
46-52: Future-proof tag filtering: clarify this builds tagFilters, not facetFiltersThis implementation dedupes labels and returns a single OR list for
tagFilters. That’s correct for_tags, but will silently ignore any other facets should they be added later. At minimum, rename for clarity; optionally, support per-facet filters viafiltersorfacetFilters.Option A (rename only):
-function mapToAlgoliaFilters(tagsByFacet) { +function mapTagsToTagFilters(tagsByFacet) { const set = new Set(); Object.values(tagsByFacet) .flat() .forEach((tag) => set.add(tag.label)); return [Array.from(set)]; }And update callers accordingly.
Option B (support facet-aware filters via filters/facetFilters) available on request.
65-84: Guard against double-initializing Search InsightsIf this partial is loaded more than once,
aa('init', ...)may be called repeatedly. You already guard script insertion; add an init guard too.window.aa = window.aa || function(){ (window.aa.q = window.aa.q || []).push(arguments); }; if (!window.__aaLoaderInserted) { var si = document.createElement('script'); si.async = 1; si.src = 'https://cdn.jsdelivr.net/npm/search-insights@2'; document.head.appendChild(si); window.__aaLoaderInserted = true; } - aa('init', { - appId: '{{{env.ALGOLIA_APP_ID}}}', - apiKey: '{{{env.ALGOLIA_API_KEY}}}', - useCookie: true - }); + if (!window.__aaInitialized) { + aa('init', { + appId: '{{{env.ALGOLIA_APP_ID}}}', + apiKey: '{{{env.ALGOLIA_API_KEY}}}', + useCookie: true + }); + window.__aaInitialized = true; + }
151-163: cURL example origin handling is safe; consider base URL fallbackGood try/catch on URL parsing. If many API records ship relative URLs, consider a configurable base (e.g., from env or page context) so the cURL example always shows a fully-qualified URL.
I can propose a small helper that reads a
{{{env.API_BASE_URL}}}when present and falls back to relative paths.
165-175: Add accessible name to the Ask AI buttonScreen readers will announce the SVG but not the action clearly. Add an explicit aria-label.
- ? html`<button + ? html`<button type="button" class="ask-ai" + aria-label=${`Ask AI about "${currentQuery}"`} onClick=${() => { if (window.Kapa?.open) { window.Kapa.open({ query: currentQuery }); autocompleteInstance.setIsOpen(false); } }} >
194-201: URL-encode version parameter in "View all results" link
VERSIONcan contain pre-release identifiers. Encode to avoid malformed URLs.- href="{{{relativize '/search' }}}?q=${encodeURIComponent(currentQuery)}${VERSION ? `&version=${VERSION}` : ''}${productParams}" + href="{{{relativize '/search' }}}?q=${encodeURIComponent(currentQuery)}${VERSION ? `&version=${encodeURIComponent(VERSION)}` : ''}${productParams}"
390-416: Facet source: remove-only behavior is good; consider excluding active tags at the query levelYou filter out already-added tags in
transformResponse. That works, but you still request all_tagsevery time. If the facet source grows, you can passtagFilterswith the selected tags to bias the facet results or limit noise.No change needed for this PR; just a future tuning suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/partials/algolia-script.hbs(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/partials/algolia-script.hbs (4)
118-145: Nice: preview stays in sync with query and filtersUsing
prevStateto detect tag changes and seeding the preview with the top docs hit is a solid UX improvement.
456-490: Fix validated: non-doc items no longer depend on unixTimestamp existenceSorting non-doc items with
(b.unixTimestamp ?? -Infinity) - (a.unixTimestamp ?? -Infinity)removes the hard requirement forunixTimestamp, which was the root cause of empty API results. Keeping docs in their ranked positions while interleaving non-doc items is a nice touch.
598-598: Good: getItemUrl prefers canonical URL with objectID fallbackThis helps route API records that actually have URLs while keeping legacy behavior for items keyed by objectID only.
32-43: No change required: theHome/Labsbranch is reachable as intendedThe
INITIAL_TAGhelper only appends a version when the page is part of a versioned component (i.e. whenpage.componentVersion.versionoris-prereleaseis true). On your un-versioned Home and Labs pages,INITIAL_TAGfalls back topage.component.title, which is exactly"Home"or"Labs". Consequently, the existingif (initialTag === 'Home' || initialTag === 'Labs') { … }will correctly fire for those pages, and no additional regex is needed.
Likely an incorrect or invalid review comment.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/layouts/search.hbs (1)
116-126: Add rel="noopener noreferrer" to external links opened in new tabsExternal anchors use target="_blank" without rel="noopener noreferrer", which enables reverse tabnabbing and incurs a small perf hit. Add rel attributes to both links in the external branch.
Apply this diff:
- <a href="${hit.objectID}" target="_blank"> + <a href="${hit.objectID}" target="_blank" rel="noopener noreferrer"> ... - <a href="${hit.objectID}"> + <a href="${hit.objectID}" target="_blank" rel="noopener noreferrer">src/layouts/labs-search.hbs (1)
131-140: Add rel="noopener noreferrer" to external links with target="_blank"Harden external links against reverse tabnabbing.
Apply this diff:
- <h2 class="ais-Heading"><a href="${hit.objectID}" target="_blank">${components.Highlight({ attribute: 'title', hit })}</a></h2> - <p><a href="${hit.objectID}">${components.Snippet({ attribute: 'intro', hit })}</a></p> + <h2 class="ais-Heading"><a href="${hit.objectID}" target="_blank" rel="noopener noreferrer">${components.Highlight({ attribute: 'title', hit })}</a></h2> + <p><a href="${hit.objectID}" target="_blank" rel="noopener noreferrer">${components.Snippet({ attribute: 'intro', hit })}</a></p>src/css/search.css (1)
369-371: Fix invalid color token usage (rgb tuple used as color)color: var(--aa-text-color-rgb) is invalid because the variable holds “38,38,39”, not a CSS color. Wrap it with rgb()/rgba().
Apply this diff:
-.aa-DetachedSearchButtonPlaceholder { - color: var(--aa-text-color-rgb) !important; - font-size: 12.5px; -} +.aa-DetachedSearchButtonPlaceholder { + color: rgba(var(--aa-text-color-rgb), var(--aa-text-color-alpha)) !important; + font-size: 12.5px; +}
🧹 Nitpick comments (10)
src/layouts/search.hbs (2)
146-158: Normalize product values before dedup to avoid false-duplicate missesCase-insensitive dedup is good. Also trim whitespace so "Cloud" and "Cloud " don’t render twice. Render the trimmed value.
Apply this diff:
- const seen = new Set(); - return hit.product.filter(p => { - const norm = (p || '').toLowerCase(); + const seen = new Set(); + return hit.product.filter(p => { + const norm = (p || '').toString().trim().toLowerCase(); if (seen.has(norm)) return false; seen.add(norm); return true; - }).map(p => html`<div class="aa-ItemContentTitle result-type">${p}</div>`); + }).map(p => { + const ptrim = (p || '').toString().trim(); + return html`<div class="aa-ItemContentTitle result-type">${ptrim}</div>`; + });
127-133: Consider rendering product chips for external hits for parityThe labs page renders product for both external and internal hits; here it’s only for internal. For a consistent UX, consider adding the same product rendering in the external branch too (if product is present on those records).
Also applies to: 144-162
src/layouts/labs-search.hbs (1)
140-152: Trim + normalize when deduplicating product chipsSame suggestion as search.hbs: trim values before case-insensitive dedup and render the trimmed value.
Apply this diff in both branches:
- const seen = new Set(); - return hit.product.filter(p => { - const norm = (p || '').toLowerCase(); + const seen = new Set(); + return hit.product.filter(p => { + const norm = (p || '').toString().trim().toLowerCase(); if (seen.has(norm)) return false; seen.add(norm); return true; - }).map(p => html`<div class="aa-ItemContentTitle result-type">${p}</div>`); + }).map(p => { + const ptrim = (p || '').toString().trim(); + return html`<div class="aa-ItemContentTitle result-type">${ptrim}</div>`; + });Also applies to: 164-176
src/css/search-bump.css (4)
57-61: Avoid fragile[style*="display: none"]selectors; use a state class.Attribute substring matching is brittle and expensive. Toggle an
.is-hiddenclass instead; it’s also friendlier to SSR/style sanitizers.CSS change:
-.aa-Panel .context-dropdown-menu[style*="display: none"] { +.aa-Panel .context-dropdown-menu.is-hidden { opacity: 0; visibility: hidden; transform: translateY(-8px); }Template change (replace inline display toggling with class toggling):
- <ul - id="algolia-filters-dropdown-menu" - class="context-dropdown-menu" + <ul + id="algolia-filters-dropdown-menu" + class="context-dropdown-menu ${dropdownOpen ? '' : 'is-hidden'}" role="menu" tabIndex="-1" ref=${el => (__dropdownRefs.menu = el)} - style="min-width:220px;max-height:320px;overflow:auto;display:${dropdownOpen ? 'block' : 'none'};padding:6px 0;" + style="min-width:220px;max-height:320px;overflow:auto;padding:6px 0;" onKeyDown=${handleDropdownKeyDown} >Also applies to: 376-383
85-86: Use the new token instead of hard-codedwhite.You already introduced
--color-white; use it for consistency and theming.- color: white; + color: var(--color-white); ... - color: white; + color: var(--color-white);Also applies to: 90-91
566-568: Remove duplicate border rule for non-filter items.This rule duplicates the earlier
li.aa-Item:not([id*="filters"])and adds CSS bloat.-li.aa-Item:not([id*=filters]) { - border-bottom: 1px solid #dadde1; -}
27-34: Optional: respect reduced motion preferences.Animations on the arrow and menu should be disabled for users with reduced motion.
Add once (near the end of the file is fine):
@media (prefers-reduced-motion: reduce) { .context-dropdown-arrow, .context-dropdown-toggle, .context-dropdown-menu { transition: none !important; transform: none !important; } }src/partials/algolia-script.hbs (3)
154-159: Initialize Search Insights once.If this script runs more than once, calling
aa('init', …)repeatedly can be noisy. Gate it.- aa('init', { - appId: '{{{env.ALGOLIA_APP_ID}}}', - apiKey: '{{{env.ALGOLIA_API_KEY}}}', - useCookie: true, // helps stitch sessions for analytics - }); + if (!window.__aaInitialized) { + aa('init', { + appId: '{{{env.ALGOLIA_APP_ID}}}', + apiKey: '{{{env.ALGOLIA_API_KEY}}}', + useCookie: true, // helps stitch sessions for analytics + }); + window.__aaInitialized = true; + }Also confirm that
{{{env.ALGOLIA_API_KEY}}}is a search-only key (not an admin key).
639-641: Defensive guard if the tags plugin isn’t attached yet.
state.context.tagsPlugincan be undefined on the very first render. Guard to avoid a crash.- const tagsByFacet = groupBy(state.context.tagsPlugin.tags, (tag) => tag.facet); + const tagsByFacet = groupBy(state.context?.tagsPlugin?.tags || [], (tag) => tag.facet);
803-806: Avoid nesting interactive elements (button inside anchor).A inside an is invalid and can cause focus/AT issues. Use a non-interactive element styled as a button.
- <button class="aa-ItemActionButton aa-DesktopOnly aa-ActiveOnly" type="button" aria-label="Open page" title="Open page"> + <span class="aa-ItemActionButton aa-DesktopOnly aa-ActiveOnly" role="img" aria-label="Open page" title="Open page"> <svg ... /> - </button> + </span>Also applies to: 853-855
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/css/doc.css(2 hunks)src/css/search-bump.css(3 hunks)src/css/search.css(4 hunks)src/css/vendor/algolia/autocomplete-theme-classic.css(1 hunks)src/layouts/labs-search.hbs(2 hunks)src/layouts/search.hbs(1 hunks)src/partials/algolia-script.hbs(3 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/css/vendor/algolia/autocomplete-theme-classic.css
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
margin is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
border-bottom is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
box-shadow is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
max-width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
max-height is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
border-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
height is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background-image is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5-5: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background-image is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (18)
src/css/doc.css (2)
228-238: Token swap for inline code blocks: confirm contrast and cross-theme consistencyYou changed inline/table code backgrounds to use var(--pre-background). Make sure var(--pre-background) provides adequate contrast in both light and dark themes and aligns with code block backgrounds elsewhere. If the intent is to differentiate inline vs block code, document that in the theme tokens to avoid future regressions.
979-986: Specificity/cascade check for code block backgroundsThese selectors now flip certain pre/code backgrounds to var(--code-background) while earlier generic rules set pre to var(--pre-background). This relies on selector specificity to win. Please verify Prism/Highlight.js rules (e.g., code[class*=language-], pre[class*=language-]) don’t override this unintentionally, especially given some rules use !important. If needed, mirror the token switch in those rules to avoid mixed backgrounds.
src/layouts/search.hbs (1)
112-135: Heuristic for “external” items is brittleUsing hit.objectID.startsWith('https') assumes objectID is a URL. In many Algolia setups, the URL is on hit.url. If your indexer ever changes objectID semantics, this breaks. Prefer hit.url (if present) for both detection and href, with a fallback to objectID.
If hit.url exists, update:
- const isExternal = hit.objectID.startsWith('https'); + const url = hit.url || hit.objectID; + const isExternal = url.startsWith('http'); ... - <a href="${hit.objectID}" target="_blank" rel="noopener noreferrer"> + <a href="${url}" target="_blank" rel="noopener noreferrer"> ... - <a href="${hit.objectID}"> + <a href="${url}">src/layouts/labs-search.hbs (1)
121-133: Prefer hit.url (when available) over objectID for link targetsAvoid coupling to objectID being a URL; it often isn’t. Use hit.url if present; fall back to objectID.
Proposed minimal change:
- const isExternal = hit.objectID.startsWith('https') ? true : false; + const url = hit.url || hit.objectID; + const isExternal = url.startsWith('http'); ... - <h2 class="ais-Heading"><a href="${hit.objectID}" target="_blank" rel="noopener noreferrer">${components.Highlight({ attribute: 'title', hit })}</a></h2> - <p><a href="${hit.objectID}" target="_blank" rel="noopener noreferrer">${components.Snippet({ attribute: 'intro', hit })}</a></p> + <h2 class="ais-Heading"><a href="${url}" target="_blank" rel="noopener noreferrer">${components.Highlight({ attribute: 'title', hit })}</a></h2> + <p><a href="${url}" target="_blank" rel="noopener noreferrer">${components.Snippet({ attribute: 'intro', hit })}</a></p> ... - <h2 class="ais-Heading"><a href="${hit.objectID}">${components.Highlight({ attribute: 'title', hit })}</a></h2> - <p><a href="${hit.objectID}">${components.Snippet({ attribute: 'intro', hit })}</a></p> + <h2 class="ais-Heading"><a href="${url}">${components.Highlight({ attribute: 'title', hit })}</a></h2> + <p><a href="${url}">${components.Snippet({ attribute: 'intro', hit })}</a></p>Also applies to: 158-166
src/css/vendor/algolia/autocomplete-theme-classic.css (1)
5-5: Stylelint/Biome duplicate-property warnings in vendor CSSStatic analysis flags duplicate properties (expected in vendored/minified CSS for fallbacks). Exclude src/css/vendor/** from lints to reduce noise and avoid churn.
You can configure your linter to ignore vendor assets (example):
- For Biome:
- Add to biome.json:
{
"linter": {
"ignore": ["src/css/vendor/**"]
}
}- Or add a .biomeignore entry:
src/css/vendor/src/css/search.css (5)
36-55: Confirm stacking context so the dropdown isn’t clippedThis menu uses z-index: 1000 inside an Algolia panel that establishes its own stacking context. It’s probably fine, but verify the menu always appears above sibling panel chrome and isn’t clipped by overflow hidden on ancestors.
1-55: Nice: context dropdown UI is compact and theme-awareGood use of existing tokens (link/body/panel variables). The transitions feel balanced and the arrow rotation tied to aria-expanded is a solid approach.
99-106: Search page layout tweaks look saneSetting the container to width: 100% and adjusting spacing/gaps simplifies the grid and aligns with the new filter UX. No issues spotted.
Also applies to: 153-163, 175-183
263-268: Header and tags source visibility changes LGTMThe horizontal header, ask-ai spacing, and hiding the tagsPlugin source align with the new UX. Clean and minimal.
Also applies to: 283-285, 286-288
413-414: Minor visual tweaksWider keyboard badge and taller detached modal are sensible. No concerns.
Also applies to: 419-420
src/css/search-bump.css (3)
289-291: Confirm intentional hiding of the tagsPlugin source.Hiding the source entirely is fine if the custom dropdown replaces it everywhere. Ensure this is not duplicated in other CSS (search.css) to avoid maintenance drift.
Would you like me to scan for duplicate selectors and consolidate them?
436-440: LGTM: taller modal improves API previews.Bumping min-height to 500px aligns with the richer preview content.
5-55: Nice addition: context dropdown container and items.The structure (relative container, toggle, arrow, menu, items) looks good and matches the new HBS behavior.
Also applies to: 63-76
src/partials/algolia-script.hbs (5)
497-503: Verify Insights “positions” accuracy.Using
state.activeItemId + 1may not correspond to Algolia’s absolute position in the results. Prefer the per-hitpositionif available (whenclickAnalytics: true), falling back toactiveItemId + 1.Proposed adjustment:
- positions: [state.activeItemId + 1], + positions: [ + state.context.preview?.position ?? + state.context.preview?.__position ?? + state.activeItemId + 1 + ],If your hits expose
__position(orposition) from Algolia, switch to that for accurate analytics.Also applies to: 541-544, 582-587
349-353: Great job on the accessible, keyboard-navigable dropdown.
- Proper roles/aria attributes
- Arrow key handling
- Clear All and Close actions
- Outside-click and Esc lifecycle scoped to open state
Also applies to: 355-422
430-438: Nice endpoint preview and cURL example.This will help users evaluate API results quickly. Good guardrails around missing origin.
51-54: Tags defaulting and plugin wiring look solid.Merging contextual tag + catalog and shaping to
{ label, facet }aligns with the tags plugin API.Also applies to: 170-193
278-285: Seeding preview from the first docs hit is a good UX improvement.The prev/next tag-key comparison avoids unnecessary updates.
|
I'm not sure if it's this PR that its making this, but the latest product version is not being correctly picked by default Not tagging it as a blocker because I can't trace where this info is coming from. Please check before merging. |
| if (seen.has(norm)) return false; | ||
| seen.add(norm); | ||
| return true; | ||
| }).map(p => html`<div class="aa-ItemContentTitle result-type">${p}</div>`); |
There was a problem hiding this comment.
mapping by div class can be dangerous as these can change with obfuscation. Consider creating a test to make sure that changes to these classes gets reported on new builds.
The UI mocks an Antora environment in page:
url: *home_url
home: true
title: Brand’s Hardware & Software Requirements
component: *component
componentVersion: *component_version
version: '24.3'
displayVersion: '24.3' |
Previously, API search results always appeared empty because we expected a unixtimestamp field.
This PR adds richer previews for API-related results and introduces a visible filter
dropdown that lets users include/exclude tag facets from the query.
Why
What’s new
Preview pane enhancements
Filter dropdown (facets UI)