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.

Sign up for GitHub

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

Jump to bottom

feat(site): add orama search #1948

Closed
wants to merge 1 commit into from
Closed

feat(site): add orama search #1948

wants to merge 1 commit into from
+309 −2

Conversation

jog1t
Copy link
Contributor

@jog1t jog1t commented Jan 28, 2025

Changes

@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

cloudflare-workers-and-pages bot commented Jan 28, 2025 •
edited
Loading

Deploying rivet with  Cloudflare Pages  Cloudflare Pages

Latest commit: fcee7b2
Status: ✅  Deploy successful!
Preview URL: https://eddb55d2.rivet.pages.dev
Branch Preview URL: https://01-28-feat-site-add-orama-se.rivet.pages.dev

View logs

@jog1t jog1t self-assigned this Jan 28, 2025
@jog1t jog1t marked this pull request as ready for review January 28, 2025 01:46
This was referenced Jan 28, 2025
Closed
Closed
@jog1t Graphite App
Copy link
Contributor Author

jog1t commented Jan 28, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jog1t jog1t mentioned this pull request Jan 28, 2025
Closed
greptile-apps[bot]
greptile-apps bot reviewed Jan 28, 2025
View reviewed changes
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added search functionality to the site header using OramaSearchBox, implementing a modal-based search interface with keyboard shortcut display.

  • Hardcoded API key in site/src/components/v2/HeaderSearch.tsx poses a security risk and should be moved to environment variables
  • Missing keyboard shortcut implementation for Cmd+K despite UI showing the shortcut in HeaderSearch.tsx
  • No error handling for failed API requests in HeaderSearch.tsx
  • Using 'latest' tag for @next/third-parties and '*' for react/framer-motion in package.json could lead to breaking changes
  • Outdated eslint-config-next (13.0.2) compared to Next.js version (^15.0) in package.json

3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

site/package.json
@@ -24,6 +24,7 @@
"@mdx-js/react": "^2.1.5",
"@next/mdx": "^14.0.1",
"@next/third-parties": "latest",
Copy link

Choose a reason for hiding this comment

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

style: using 'latest' tag can lead to unexpected breaking changes in production. Consider pinning to a specific version

site/src/components/v2/HeaderSearch.tsx
index={{
endpoint:
"https://cloud.orama.run/v1/indexes/rivet-gg-b87fiw",
api_key: "dcVm1fAKZeTdOfGFZCCH9xWiH7JeYCQZ",
Copy link

Choose a reason for hiding this comment

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

logic: API key should not be hardcoded in the source code. Move this to an environment variable.

site/src/components/v2/HeaderSearch.tsx
Comment on lines +19 to +21
<Kbd className="absolute right-[0.3rem] top-[0.3rem] hidden sm:flex">
<Kbd.Cmd />K
</Kbd>
Copy link

Choose a reason for hiding this comment

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

logic: Keyboard shortcut (Cmd+K) is displayed but no keyboard event handler is implemented to handle this shortcut

@NathanFlurry NathanFlurry force-pushed the 01-28-feat_hub_display_onboarding_only_when_user_has_no_builds branch from 4b5dfcf to e57ca0d Compare January 28, 2025 05:40
@NathanFlurry NathanFlurry force-pushed the 01-28-feat_site_add_orama_search branch from 3670f55 to fcee7b2 Compare January 28, 2025 05:40
@graphite-app Graphite App
Copy link
Contributor

graphite-app bot commented Jan 28, 2025 •
edited
Loading

Merge activity

  • Jan 28, 12:47 AM EST: A user added this pull request to the Graphite merge queue.
  • Jan 28, 12:48 AM EST: CI is running for this PR on a draft PR: #1949
  • Jan 28, 12:49 AM EST: A user merged this pull request with the Graphite merge queue via draft PR: #1949.

NathanFlurry pushed a commit that referenced this pull request Jan 28, 2025
@jog1t
feat(site): add orama search (#1948)
ac5de3e 
<!-- Please make sure there is an issue that this PR is correlated to. -->

## Changes

<!-- If there are frontend changes, please include screenshots. -->
@graphite-app graphite-app bot closed this Jan 28, 2025
@graphite-app graphite-app bot deleted the 01-28-feat_site_add_orama_search branch January 28, 2025 05:49
@github-actions github-actions bot mentioned this pull request Jan 27, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@NicholasKissel NicholasKissel NicholasKissel approved these changes

@NathanFlurry NathanFlurry Awaiting requested review from NathanFlurry

Assignees

@jog1t jog1t

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jog1t @NicholasKissel