-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
move embedding options to their own component #387
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
Conversation
frontend/src/components/EmbeddingSelection/OpenAiOptions/index.jsx, frontend/src/components/EmbeddingSelection/AzureAiOptions/index.jsx, frontend/src/components/EmbeddingSelection/LocalAiOptions/index.jsxConsider adding PropTypes to the new components (OpenAiOptions, AzureAiOptions, LocalAiOptions) to enforce the type of props they should receive. This will help catch errors during development and improve the robustness of the code. import PropTypes from 'prop-types';
OpenAiOptions.propTypes = {
settings: PropTypes.object.isRequired,
};
AzureAiOptions.propTypes = {
settings: PropTypes.object.isRequired,
};
LocalAiOptions.propTypes = {
settings: PropTypes.object.isRequired,
};Consider using React.memo for the new components (OpenAiOptions, AzureAiOptions, LocalAiOptions) to prevent unnecessary re-renders if the props do not change. This can improve performance, especially for larger components. export default React.memo(OpenAiOptions);
export default React.memo(AzureAiOptions);
export default React.memo(LocalAiOptions);frontend/src/pages/GeneralSettings/EmbeddingPreference/index.jsxConsider using the React.memo function to wrap the new components (OpenAiOptions, AzureAiOptions, LocalAiOptions). This will prevent unnecessary re-renders of these components when the props they receive do not change, thus improving the performance of your application. export default React.memo(OpenAiOptions);
export default React.memo(AzureAiOptions);
export default React.memo(LocalAiOptions);Consider using a switch statement or a lookup object instead of multiple if-else statements when rendering different components based on the value of 'embeddingChoice'. This will make the code cleaner and easier to read. const embeddingOptions = {
'openai': <OpenAiOptions settings={settings} />,
'azure': <AzureAiOptions settings={settings} />,
'localai': <LocalAiOptions settings={settings} />
};
return (
<div className="mt-10 flex flex-wrap gap-4 max-w-[800px]">
{embeddingOptions[embeddingChoice]}
</div>
); |
| <div className="mt-10 flex flex-wrap gap-4 max-w-[800px]"> | ||
| {embeddingChoice === "openai" && ( | ||
| <> | ||
| <div className="flex flex-col w-60"> | ||
| <label className="text-white text-sm font-semibold block mb-4"> | ||
| API Key | ||
| </label> | ||
| <input | ||
| type="text" | ||
| name="OpenAiKey" | ||
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | ||
| placeholder="OpenAI API Key" | ||
| defaultValue={ | ||
| settings?.OpenAiKey ? "*".repeat(20) : "" | ||
| } | ||
| required={true} | ||
| autoComplete="off" | ||
| spellCheck={false} | ||
| /> | ||
| </div> | ||
| <div className="flex flex-col w-60"> | ||
| <label className="text-white text-sm font-semibold block mb-4"> | ||
| Model Preference | ||
| </label> | ||
| <select | ||
| disabled={true} | ||
| className="cursor-not-allowed bg-zinc-900 border border-gray-500 text-white text-sm rounded-lg block w-full p-2.5" | ||
| > | ||
| <option disabled={true} selected={true}> | ||
| text-embedding-ada-002 | ||
| </option> | ||
| </select> | ||
| </div> | ||
| </> | ||
| <OpenAiOptions settings={settings} /> | ||
| )} | ||
|
|
||
| {embeddingChoice === "azure" && ( | ||
| <> | ||
| <div className="flex flex-col w-60"> | ||
| <label className="text-white text-sm font-semibold block mb-4"> | ||
| Azure Service Endpoint | ||
| </label> | ||
| <input | ||
| type="url" | ||
| name="AzureOpenAiEndpoint" | ||
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | ||
| placeholder="https://my-azure.openai.azure.com" | ||
| defaultValue={settings?.AzureOpenAiEndpoint} | ||
| required={true} | ||
| autoComplete="off" | ||
| spellCheck={false} | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="flex flex-col w-60"> | ||
| <label className="text-white text-sm font-semibold block mb-4"> | ||
| API Key | ||
| </label> | ||
| <input | ||
| type="password" | ||
| name="AzureOpenAiKey" | ||
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | ||
| placeholder="Azure OpenAI API Key" | ||
| defaultValue={ | ||
| settings?.AzureOpenAiKey ? "*".repeat(20) : "" | ||
| } | ||
| required={true} | ||
| autoComplete="off" | ||
| spellCheck={false} | ||
| /> | ||
| </div> | ||
|
|
||
| <div className="flex flex-col w-60"> | ||
| <label className="text-white text-sm font-semibold block mb-4"> | ||
| Embedding Deployment Name | ||
| </label> | ||
| <input | ||
| type="text" | ||
| name="AzureOpenAiEmbeddingModelPref" | ||
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | ||
| placeholder="Azure OpenAI embedding model deployment name" | ||
| defaultValue={settings?.AzureOpenAiEmbeddingModelPref} | ||
| required={true} | ||
| autoComplete="off" | ||
| spellCheck={false} | ||
| /> | ||
| </div> | ||
| </> | ||
| <AzureAiOptions settings={settings} /> | ||
| )} | ||
|
|
||
| {embeddingChoice === "localai" && ( | ||
| <> | ||
| <div className="flex flex-col w-60"> | ||
| <label className="text-white text-sm font-semibold block mb-4"> | ||
| LocalAI Base URL | ||
| </label> | ||
| <input | ||
| type="url" | ||
| name="EmbeddingBasePath" | ||
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | ||
| placeholder="http://localhost:8080/v1" | ||
| defaultValue={settings?.EmbeddingBasePath} | ||
| onChange={(e) => setBasePathValue(e.target.value)} | ||
| onBlur={updateBasePath} | ||
| required={true} | ||
| autoComplete="off" | ||
| spellCheck={false} | ||
| /> | ||
| </div> | ||
| <LocalAIModelSelection | ||
| settings={settings} | ||
| basePath={basePath} | ||
| /> | ||
| </> | ||
| <LocalAiOptions settings={settings} /> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor the if-else statements to a lookup object for rendering different components based on the value of 'embeddingChoice'. This will make the code cleaner and easier to read.
| <div className="mt-10 flex flex-wrap gap-4 max-w-[800px]"> | |
| {embeddingChoice === "openai" && ( | |
| <> | |
| <div className="flex flex-col w-60"> | |
| <label className="text-white text-sm font-semibold block mb-4"> | |
| API Key | |
| </label> | |
| <input | |
| type="text" | |
| name="OpenAiKey" | |
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | |
| placeholder="OpenAI API Key" | |
| defaultValue={ | |
| settings?.OpenAiKey ? "*".repeat(20) : "" | |
| } | |
| required={true} | |
| autoComplete="off" | |
| spellCheck={false} | |
| /> | |
| </div> | |
| <div className="flex flex-col w-60"> | |
| <label className="text-white text-sm font-semibold block mb-4"> | |
| Model Preference | |
| </label> | |
| <select | |
| disabled={true} | |
| className="cursor-not-allowed bg-zinc-900 border border-gray-500 text-white text-sm rounded-lg block w-full p-2.5" | |
| > | |
| <option disabled={true} selected={true}> | |
| text-embedding-ada-002 | |
| </option> | |
| </select> | |
| </div> | |
| </> | |
| <OpenAiOptions settings={settings} /> | |
| )} | |
| {embeddingChoice === "azure" && ( | |
| <> | |
| <div className="flex flex-col w-60"> | |
| <label className="text-white text-sm font-semibold block mb-4"> | |
| Azure Service Endpoint | |
| </label> | |
| <input | |
| type="url" | |
| name="AzureOpenAiEndpoint" | |
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | |
| placeholder="https://my-azure.openai.azure.com" | |
| defaultValue={settings?.AzureOpenAiEndpoint} | |
| required={true} | |
| autoComplete="off" | |
| spellCheck={false} | |
| /> | |
| </div> | |
| <div className="flex flex-col w-60"> | |
| <label className="text-white text-sm font-semibold block mb-4"> | |
| API Key | |
| </label> | |
| <input | |
| type="password" | |
| name="AzureOpenAiKey" | |
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | |
| placeholder="Azure OpenAI API Key" | |
| defaultValue={ | |
| settings?.AzureOpenAiKey ? "*".repeat(20) : "" | |
| } | |
| required={true} | |
| autoComplete="off" | |
| spellCheck={false} | |
| /> | |
| </div> | |
| <div className="flex flex-col w-60"> | |
| <label className="text-white text-sm font-semibold block mb-4"> | |
| Embedding Deployment Name | |
| </label> | |
| <input | |
| type="text" | |
| name="AzureOpenAiEmbeddingModelPref" | |
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | |
| placeholder="Azure OpenAI embedding model deployment name" | |
| defaultValue={settings?.AzureOpenAiEmbeddingModelPref} | |
| required={true} | |
| autoComplete="off" | |
| spellCheck={false} | |
| /> | |
| </div> | |
| </> | |
| <AzureAiOptions settings={settings} /> | |
| )} | |
| {embeddingChoice === "localai" && ( | |
| <> | |
| <div className="flex flex-col w-60"> | |
| <label className="text-white text-sm font-semibold block mb-4"> | |
| LocalAI Base URL | |
| </label> | |
| <input | |
| type="url" | |
| name="EmbeddingBasePath" | |
| className="bg-zinc-900 text-white placeholder-white placeholder-opacity-60 text-sm rounded-lg focus:border-white block w-full p-2.5" | |
| placeholder="http://localhost:8080/v1" | |
| defaultValue={settings?.EmbeddingBasePath} | |
| onChange={(e) => setBasePathValue(e.target.value)} | |
| onBlur={updateBasePath} | |
| required={true} | |
| autoComplete="off" | |
| spellCheck={false} | |
| /> | |
| </div> | |
| <LocalAIModelSelection | |
| settings={settings} | |
| basePath={basePath} | |
| /> | |
| </> | |
| <LocalAiOptions settings={settings} /> | |
| )} | |
| const embeddingOptions = { | |
| 'openai': <OpenAiOptions settings={settings} />, | |
| 'azure': <AzureAiOptions settings={settings} />, | |
| 'localai': <LocalAiOptions settings={settings} /> | |
| }; | |
| return ( | |
| <div className="mt-10 flex flex-wrap gap-4 max-w-[800px]"> | |
| {embeddingOptions[embeddingChoice]} | |
| </div> | |
| ); |
No description provided.