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: adds OSS Orama support #39

Merged
merged 1 commit into from
Nov 25, 2024
Merged

feat: adds OSS Orama support #39

merged 1 commit into from
Nov 25, 2024

Conversation

micheleriva
Copy link
Member

@micheleriva micheleriva commented Sep 26, 2024 •
edited by rjborba
Loading

Adds OSS Orama Support as option of Client Instance

@vercel Vercel
Copy link

vercel bot commented Sep 26, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
orama-ui-components-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 25, 2024 4:14pm

@rjborba rjborba force-pushed the feat/adds-orama-oss-support branch from e9964b1 to 7a3674d Compare October 7, 2024 21:34
@rjborba rjborba marked this pull request as ready for review October 7, 2024 21:34
@raiindev
Copy link
Contributor

LGTM!

@rjborba rjborba force-pushed the feat/adds-orama-oss-support branch from 7a3674d to 53a204d Compare October 17, 2024 12:50
rjborba
rjborba approved these changes Oct 31, 2024
View reviewed changes
apps/storybook/stories/config.ts
@@ -28,11 +28,6 @@ const demoIndexes: DemoIndexConfig = {
description: 'content',
section: 'category',
},
highlight: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Forgot to remove it. Thanks! You could also use highlightTitle or highlightDescription

@raiindev
Copy link
Contributor

With commit c638a16, I've added some workarounds to make OSS Chat work.
The first one is related to interaction.sources being an object when the question in the chat returns no results.
Since it changes structure, I just added a ternary operator.

The second one is related to the ask method arguments; I've noticed that the OSS instance does not support some of the passed arguments, so I added a condition to pass those only in Cloud mode.

@raiindev raiindev force-pushed the feat/adds-orama-oss-support branch from 7dddf5f to 5453786 Compare November 22, 2024 11:01
rjborba
rjborba reviewed Nov 22, 2024
View reviewed changes
packages/ui-stencil/src/components.d.ts Outdated
@@ -58,7 +58,7 @@ export namespace Components {
}
interface OramaChatBox {
"autoFocus": boolean;
"clientInstance"?: OramaClient;
"clientInstance"?: OramaClient | Orama<unknown>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we be using OramaSwtich?

Copy link
Contributor

@raiindev raiindev Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The clientInstance coming as a prop should be an OramaClient | AnyOrama.
The Switch is instantiated inside the ChatService / SearchService.
Ex.
ChatService
SearchService

rjborba
rjborba reviewed Nov 22, 2024
View reviewed changes
packages/ui-stencil/src/components.d.ts
@@ -133,7 +133,7 @@ export namespace Components {
}
interface OramaSearchBox {
"chatPlaceholder"?: string;
"clientInstance"?: OramaClient;
"clientInstance"?: OramaClient | AnyOrama;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we're usingAnyOrama here and Oraam on OramaChatBox?

rjborba
rjborba reviewed Nov 22, 2024
View reviewed changes
packages/ui-stencil/src/components.d.ts Outdated
@@ -517,7 +517,7 @@ declare namespace LocalJSX {
}
interface OramaChatBox {
"autoFocus"?: boolean;
"clientInstance"?: OramaClient;
"clientInstance"?: OramaClient | Orama<unknown>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as previous comment

rjborba
rjborba reviewed Nov 22, 2024
View reviewed changes
packages/ui-stencil/src/components.d.ts
@@ -594,7 +594,7 @@ declare namespace LocalJSX {
}
interface OramaSearchBox {
"chatPlaceholder"?: string;
"clientInstance"?: OramaClient;
"clientInstance"?: OramaClient | AnyOrama;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as previous comment

rjborba
rjborba reviewed Nov 22, 2024
View reviewed changes
packages/ui-stencil/src/components/orama-chat-box/orama-chat-box.tsx Outdated
@@ -14,7 +15,7 @@ import '@phosphor-icons/webcomponents/dist/icons/PhArrowClockwise.mjs'
export class ChatBox {
@Element() el: HTMLElement
@Prop() index?: CloudIndexConfig
@Prop() clientInstance?: OramaClient
@Prop() clientInstance?: OramaClient | Orama<unknown>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we be using OramaSwitch?

@raiindev raiindev force-pushed the feat/adds-orama-oss-support branch from 5453786 to 1d7efea Compare November 25, 2024 09:33
@raiindev raiindev force-pushed the feat/adds-orama-oss-support branch from 1d7efea to 90dff7b Compare November 25, 2024 12:36
@raiindev raiindev force-pushed the feat/adds-orama-oss-support branch from c69f976 to eac8041 Compare November 25, 2024 15:51
@raiindev raiindev force-pushed the feat/adds-orama-oss-support branch from 752d38d to 0ea00b1 Compare November 25, 2024 16:10
@raiindev raiindev merged commit 2216a99 into main Nov 25, 2024
3 checks passed
@raiindev raiindev deleted the feat/adds-orama-oss-support branch November 25, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raiindev raiindev raiindev approved these changes

@rjborba rjborba rjborba approved these changes

@faustoq faustoq faustoq left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@micheleriva @raiindev @faustoq @rjborba