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

DocSearch Integration #342

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

Conversation

EthanRin
Copy link

@EthanRin EthanRin commented Dec 11, 2024

Description

I integrated Algolia DocSearch into SplashKit website. The relevant API keys are stored as a enviroment variables and called in the astro.config.mjs

Changes made:

  1. Integrated astrojs/starlight-docsearch plugin.
  2. Update the astro.config.mjs to include the DocSearch configuration.
  3. Created new environment variables for the DocSearch APIs.
  4. Modified package.json and package-lock.json to incorporate the astrojs/starlight-docsearch dependency.

Dependencies

  • Added astrojs/starlight-docsearch as a new dependency.

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Testing Checklist

  • Tested in latest Chrome
  • npm run build
  • npm run preview

If modified config files

  • I have checked the following files for changes:
    • astro.config.mjs
    • package.json

Folders and Files Added/Modified

  • Modified:
    • astro.config.mjs
    • package.json

Copy link

netlify bot commented Dec 11, 2024

Deploy Preview for splashkit-io failed.

Name Link
🔨 Latest commit 40bcaa9
🔍 Latest deploy log https://app.netlify.com/sites/splashkit-io/deploys/6762c54048091c000837bd95

Copy link

@ShaunR1991 ShaunR1991 left a comment

Choose a reason for hiding this comment

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

Great addition to the website - looking forward to seeing who much better this feels compared to the standard search engine. A few minor points to consider:

  • There’s a small typo in the environment variable DOCSEARCH_API_SERACH_KEY (should be DOCSEARCH_API_SEARCH_KEY).

  • Consider adding a check to ensure the required environment variables are defined before attempting to use them. This will help catch misconfigurations early.

if (!DOCSEARCH_API_ID || !DOCSEARCH_API_SEARCH_KEY || !DOCSEARCH_INDEX_NAME) { console.error("Algolia DocSearch environment variables are not set. Please check your configuration."); process.exit(1); // Exit the process to prevent incomplete setup }

  • What testing have you done of the search functionality? While the integration seems correct, have you validated that the results are displayed as expected, can handle edge cases (such as empty or really long search strings), and can update dynamically without page reloads?

  • Have you got supporting documentation for longevity of the capability? For example, have you documented the need for setting up environment variables, or if there are any specific permissions required for the API keys?

Aside from that, this looks great. Nice work!

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.

2 participants