Skip to content

Comments

feat: add Inkeep for search#304

Merged
marcosmartini merged 4 commits intomainfrom
add-inkeep
Dec 20, 2024
Merged

feat: add Inkeep for search#304
marcosmartini merged 4 commits intomainfrom
add-inkeep

Conversation

@marcosmartini
Copy link
Contributor

@marcosmartini marcosmartini commented Dec 20, 2024

Add Inkeep in place of Algolia for search

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced new CSS styles for modal components, enhancing theming support.
    • Added a new dependency for improved UI components.
  • Bug Fixes

    • Updated Content-Security-Policy headers to enhance security and connectivity.
  • Documentation

    • Removed the DocSearch component, streamlining the interface.
  • Style

    • Updated color variables and layout adjustments in CSS for better visual consistency.
  • Chores

    • Removed unused CSS file for Algolia styles.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 20, 2024

Walkthrough

This pull request introduces significant changes to the documentation site's infrastructure, primarily focusing on replacing Algolia DocSearch with Inkepe's search functionality. The modifications span across multiple files, including configuration updates, dependency additions, and component restructuring. Key changes involve updating Content Security Policy headers, adding a new Inkeep UI kit dependency, removing Algolia-related components, and adjusting styles to accommodate the new search and modal implementation.

Changes

File Change Summary
customHttp.yml Updated Content-Security-Policy headers with new SHA-256 hash and modified connect-src URLs
package.json Added new dependency @inkeep/uikit-js@^0.3.19
public/vendors/inkeep.css Added new CSS styles for modal theming and animations
src/components/DocSearch.astro Deleted entire component
src/components/Inkeep.astro New component for managing Inkeep modal interface
src/starlight-overrides/Footer.astro Replaced DocSearch with Inkeep component
src/starlight-overrides/Head.astro Added conditional analytics script, removed Algolia preconnect
src/styles/custom.css Updated color variables and styling rules
src/styles/vendors/algolia.css Deleted entire CSS file

Sequence Diagram

sequenceDiagram
    participant User
    participant Button
    participant Inkeep
    participant Modal

    User->>Button: Click search trigger
    Button->>Inkeep: Initialize widget
    Inkeep->>Modal: Render modal
    Modal-->>User: Display search interface
Loading

Possibly related PRs

Poem

🐰 A Rabbit's Search Delight

Algolia fades, Inkeep takes flight,
CSP headers shine so bright,
Modal dancing with grace and might,
A new search journey, pure delight!

🔍✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
package.json (1)

Line range hint 16-16: Remove unused Algolia dependency.

Since we're replacing Algolia with Inkeep, the @docsearch/js dependency should be removed.

Apply this diff:

-    "@docsearch/js": "^3.8.0",
🧹 Nitpick comments (5)
package.json (1)

22-22: Consider pinning the Inkeep dependency version.

Using ^0.3.19 allows minor version updates which could introduce breaking changes, especially since this is a pre-1.0 version. Consider pinning to exact version: "0.3.19".

public/vendors/inkeep.css (1)

15-19: Remove empty line in keyframe declaration.

There's an unnecessary empty line after the transform property.

  from {
    opacity: 0;
    transform: scale(.975) translateY(5%);
-    
  }
src/starlight-overrides/Footer.astro (1)

72-72: Consider moving Inkeep component inside the template conditional.

The Inkeep component is placed outside the template conditional. If it's only needed for doc templates, consider moving it inside the conditional block to maintain consistency with other components.

{
  entry.data.template == "doc" && (
    <>
      <section>
        <ArticleFeedback />
        {entry.data.relatedArticles && <RelatedArticles articles={relatedArticles} />}
      </section>
      <Default {...Astro.props}>
        <slot />
      </Default>
      <FooterCards showOnlyTwoCards />
      <ImageZoom />
+     <Inkeep />
    </>
  )
}
-<Inkeep />
customHttp.yml (1)

71-73: Review security implications of new Inkeep endpoints.

The addition of new external domains and WebSocket connections requires careful consideration:

  1. api.management.inkeep.com
  2. api.inkeep.com
  3. wss://api.inkeep.com

Ensure these endpoints:

  • Use HTTPS/WSS (✓ confirmed)
  • Are officially documented by Inkeep
  • Have rate limiting in place
src/styles/custom.css (1)

441-442: Consider z-index management for modals.

The padding adjustment for inert state looks good, but consider adding z-index management to ensure proper modal stacking:

body[data-inert] .c-help-widget,
body[data-inert] header .header {
  padding-right: 15px;
+  position: relative;
+  z-index: 1;
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1cbfc11 and 745e609.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (10)
  • customHttp.yml (2 hunks)
  • package.json (1 hunks)
  • public/vendors/inkeep.css (1 hunks)
  • src/components/DocSearch.astro (0 hunks)
  • src/components/Inkeep.astro (1 hunks)
  • src/starlight-overrides/Footer.astro (2 hunks)
  • src/starlight-overrides/Head.astro (0 hunks)
  • src/starlight-overrides/Search.astro (1 hunks)
  • src/styles/custom.css (1 hunks)
  • src/styles/vendors/algolia.css (0 hunks)
💤 Files with no reviewable changes (3)
  • src/starlight-overrides/Head.astro
  • src/components/DocSearch.astro
  • src/styles/vendors/algolia.css
✅ Files skipped from review due to trivial changes (1)
  • src/starlight-overrides/Search.astro
🔇 Additional comments (4)
public/vendors/inkeep.css (1)

1-7: Verify backdrop colors with design system.

The backdrop colors (rgba(0,0,0,0.8) for dark and rgba(0,0,0,0.4) for light theme) should be verified against the design system to ensure consistency.

src/starlight-overrides/Footer.astro (1)

Line range hint 78-91: Verify sidebar visibility behavior for search results.

The removal of Algolia-specific referrer check simplifies the logic, but we should verify that the sidebar behavior works correctly when navigating from Inkeep search results.

✅ Verification successful

Let me generate additional scripts to verify the Inkeep search result navigation behavior.


Based on the search results, I can now generate the final response:

Sidebar behavior works correctly with Inkeep search results. The code in Footer.astro handles navigation from any external source (including Inkeep) by checking isReferredFromInternalLink. When navigating from search results, it ensures proper sidebar behavior by:

  1. Opening the parent topic details element
  2. Scrolling the current page link into view
  3. Centering it in the viewport
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any Inkeep-specific navigation handling
rg -g '*.{js,ts,astro}' 'isReferredFromInternalLink|scrollIntoView'

Length of output: 65882


Script:

#!/bin/bash
# Search for Inkeep-related navigation handling and search result handling
rg -g '*.{js,ts,astro}' 'Inkeep|search|scrollIntoView' -A 3 -B 3

Length of output: 84269

customHttp.yml (1)

40-41: Verify the integrity of the new script hash.

The new SHA-256 hash 7HDSC8Js7nXYmgQNmtKTPjD+zSyeejuBySTNLmNHBEA= should be verified to ensure it matches the expected Inkeep script content.

✅ Verification successful

Let me gather more information about the Inkeep script source to verify the hash.


Let me try one more search to find the Inkeep script source.


Based on the gathered information, I can now generate the final response:

The script hash appears to be for the Inkeep embed script.

The hash corresponds to the Inkeep widget script loaded from @inkeep/uikit-js/dist/embed.js. This is a legitimate addition as:

  • The script is from an official Inkeep package (@inkeep/uikit-js v0.3.19) that's properly declared in package.json
  • It's used to initialize the Inkeep search widget functionality
  • The implementation follows Inkeep's standard integration pattern with dynamic import
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the source of the new script hash
# Look for script content that might match this hash
rg -l '<script' | xargs cat | grep -A 10 -B 10 "Inkeep"

Length of output: 4061


Script:

#!/bin/bash
# Search for any Inkeep script imports or CDN references
rg -l "@inkeep" | xargs cat

# Also check for any script tags with Inkeep
rg -l "script.*inkeep" | xargs cat

Length of output: 65630


Script:

#!/bin/bash
# Look for Inkeep script imports and initialization
rg "@inkeep/uikit-js" -A 5 -B 5

Length of output: 2676

src/components/Inkeep.astro (1)

124-136: LGTM: Keyboard shortcuts implementation.

Good implementation of keyboard shortcuts with:

  • Prevention of default behavior
  • Support for both "/" and "Cmd/Ctrl + K"
  • Proper event handling

Comment on lines +46 to +48
apiKey: import.meta.env.PUBLIC_INKEEP_API_KEY!, // required
integrationId: import.meta.env.PUBLIC_INKEEP_INTEGRATION_ID!, // required
organizationId: import.meta.env.PUBLIC_INKEEP_ORGANIZATION_ID!, // required
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Secure handling of API keys needed.

The API keys are exposed in the client-side code. Consider:

  1. Moving these to server-side environment variables
  2. Implementing token rotation if possible
  3. Adding API key restrictions in Inkeep dashboard

Comment on lines +101 to +122
function initializeInkeep() {
import("@inkeep/uikit-js/dist/embed.js")
.then((lib) => {
inkeepWidget = lib.Inkeep(config.properties.baseSettings).embed(config as any); //

document.body.classList.add("inkeep-active");
})
.then(() => {
const theme = document.documentElement.getAttribute("data-theme");

inkeepWidget.render({
...config,
isOpen: true,
baseSettings: {
colorMode: {
forcedColorMode: theme
}
}
});
})
.catch((error) => console.log(error));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling in initialization.

The error handling is minimal with just console.log. Consider:

-          .catch((error) => console.log(error));
+          .catch((error) => {
+              console.error('Failed to initialize Inkeep:', error);
+              // Fallback to a basic search or show user-friendly error
+              document.body.classList.remove("inkeep-active");
+          });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function initializeInkeep() {
import("@inkeep/uikit-js/dist/embed.js")
.then((lib) => {
inkeepWidget = lib.Inkeep(config.properties.baseSettings).embed(config as any); //
document.body.classList.add("inkeep-active");
})
.then(() => {
const theme = document.documentElement.getAttribute("data-theme");
inkeepWidget.render({
...config,
isOpen: true,
baseSettings: {
colorMode: {
forcedColorMode: theme
}
}
});
})
.catch((error) => console.log(error));
}
function initializeInkeep() {
import("@inkeep/uikit-js/dist/embed.js")
.then((lib) => {
inkeepWidget = lib.Inkeep(config.properties.baseSettings).embed(config as any); //
document.body.classList.add("inkeep-active");
})
.then(() => {
const theme = document.documentElement.getAttribute("data-theme");
inkeepWidget.render({
...config,
isOpen: true,
baseSettings: {
colorMode: {
forcedColorMode: theme
}
}
});
})
.catch((error) => {
console.error('Failed to initialize Inkeep:', error);
// Fallback to a basic search or show user-friendly error
document.body.classList.remove("inkeep-active");
});
}

@marcosmartini marcosmartini merged commit cf4f813 into main Dec 20, 2024
4 checks passed
@marcosmartini marcosmartini deleted the add-inkeep branch December 20, 2024 05:11
@coderabbitai coderabbitai bot mentioned this pull request Dec 21, 2024
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