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

back button + popups #2707

Merged
merged 2 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions web/src/app/admin/assistants/AssistantEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export function AssistantEditor({
admin?: boolean;
}) {
const router = useRouter();

const { popup, setPopup } = usePopup();

const colorOptions = [
Expand Down
9 changes: 9 additions & 0 deletions web/src/app/admin/configuration/search/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ export interface EmbeddingDetails {
default_model_id?: number;
name: string;
}

import { EmbeddingIcon } from "@/components/icons/icons";
import { usePopupFromQuery } from "@/components/popup/PopupFromQuery";

import Link from "next/link";
import { SavedSearchSettings } from "../../embeddings/interfaces";
Expand All @@ -29,6 +31,12 @@ import { SettingsContext } from "@/components/settings/SettingsProvider";

function Main() {
const settings = useContext(SettingsContext);
const { popup: searchSettingsPopup } = usePopupFromQuery({
"search-settings": {
message: `Changed search settings successfully`,
type: "success",
},
});
const {
data: currentEmeddingModel,
isLoading: isLoadingCurrentModel,
Expand Down Expand Up @@ -74,6 +82,7 @@ function Main() {

return (
<div className="h-screen">
{searchSettingsPopup}
{!futureEmbeddingModel ? (
<>
{settings?.settings.needs_reindexing && (
Expand Down
12 changes: 4 additions & 8 deletions web/src/app/admin/connectors/[connector]/AddConnectorPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { Formik } from "formik";
import { AccessTypeForm } from "@/components/admin/connectors/AccessTypeForm";
import { AccessTypeGroupSelector } from "@/components/admin/connectors/AccessTypeGroupSelector";
import NavigationRow from "./NavigationRow";

import { useRouter } from "next/navigation";
export interface AdvancedConfig {
refreshFreq: number;
pruneFreq: number;
Expand Down Expand Up @@ -111,6 +111,8 @@ export default function AddConnector({
}: {
connector: ConfigurableSources;
}) {
const router = useRouter();

// State for managing credentials and files
const [currentCredential, setCurrentCredential] =
useState<Credential<any> | null>(null);
Expand Down Expand Up @@ -201,13 +203,7 @@ export default function AddConnector({
};

const onSuccess = () => {
setPopup({
message: "Connector created! Redirecting to connector home page",
type: "success",
});
setTimeout(() => {
window.open("/admin/indexing/status", "_self");
}, 1000);
router.push("/admin/indexing/status?message=connector-created");
};

return (
Expand Down
15 changes: 3 additions & 12 deletions web/src/app/admin/embeddings/pages/EmbeddingFormPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import RerankingDetailsForm from "../RerankingFormPage";
import { useEmbeddingFormContext } from "@/components/context/EmbeddingContext";
import { Modal } from "@/components/Modal";

import { useRouter } from "next/navigation";
export default function EmbeddingForm() {
const { formStep, nextFormStep, prevFormStep } = useEmbeddingFormContext();
const { popup, setPopup } = usePopup();
const router = useRouter();

const [advancedEmbeddingDetails, setAdvancedEmbeddingDetails] =
useState<AdvancedSearchConfiguration>({
Expand Down Expand Up @@ -172,10 +174,6 @@ export default function EmbeddingForm() {

const response = await updateSearchSettings(values);
if (response.ok) {
setPopup({
message: "Updated search settings successfully",
type: "success",
});
return true;
} else {
setPopup({ message: "Failed to update search settings", type: "error" });
Expand All @@ -184,14 +182,7 @@ export default function EmbeddingForm() {
};

const navigateToEmbeddingPage = (changedResource: string) => {
setPopup({
message: `Changed ${changedResource} successfully. Redirecting to embedding page`,
type: "success",
});

setTimeout(() => {
window.open("/admin/configuration/search", "_self");
}, 2000);
router.push("/admin/configuration/search?message=search-settings");
};

const onConfirm = async () => {
Expand Down
18 changes: 14 additions & 4 deletions web/src/app/admin/indexing/status/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ import { AdminPageTitle } from "@/components/admin/Title";
import Link from "next/link";
import { Button, Text } from "@tremor/react";
import { useConnectorCredentialIndexingStatus } from "@/lib/hooks";
import { usePopupFromQuery } from "@/components/popup/PopupFromQuery";

function Main() {
const { popup } = usePopupFromQuery({
"connector-created": {
message: "Connector created successfully",
type: "success",
},
});
const {
data: indexAttemptData,
isLoading: indexAttemptIsLoading,
Expand Down Expand Up @@ -67,10 +74,13 @@ function Main() {
});

return (
<CCPairIndexingStatusTable
ccPairsIndexingStatuses={indexAttemptData}
editableCcPairsIndexingStatuses={editableIndexAttemptData}
/>
<>
{popup}
<CCPairIndexingStatusTable
ccPairsIndexingStatuses={indexAttemptData}
editableCcPairsIndexingStatuses={editableIndexAttemptData}
/>
</>
);
}

Expand Down
9 changes: 8 additions & 1 deletion web/src/components/context/EmbeddingContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,16 @@ export const EmbeddingFormProvider: React.FC<{
useEffect(() => {
// Update URL when formStep changes
const updatedSearchParams = new URLSearchParams(searchParams.toString());
const existingStep = updatedSearchParams.get("step");
updatedSearchParams.set("step", formStep.toString());
const newUrl = `${pathname}?${updatedSearchParams.toString()}`;
router.push(newUrl);

console.log("existingStep", existingStep);
Copy link

Choose a reason for hiding this comment

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

style: Remove this console.log before merging to production

if (!existingStep) {
router.replace(newUrl);
} else if (newUrl !== pathname) {
router.push(newUrl);
}
Comment on lines +64 to +68
Copy link

Choose a reason for hiding this comment

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

logic: This logic may cause unexpected behavior if the URL contains other query parameters

}, [formStep, router, pathname, searchParams]);

// Update formStep when URL changes
Expand Down
10 changes: 8 additions & 2 deletions web/src/components/context/FormContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,18 @@ export const FormProvider: React.FC<{
useEffect(() => {
// Update URL when formStep changes
const updatedSearchParams = new URLSearchParams(searchParams.toString());
const existingStep = updatedSearchParams.get("step");
updatedSearchParams.set("step", formStep.toString());
const newUrl = `${pathname}?${updatedSearchParams.toString()}`;
router.push(newUrl);

if (!existingStep) {
router.replace(newUrl);
console.log("replacing", newUrl);
Copy link

Choose a reason for hiding this comment

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

style: Remove this console.log statement before merging to production

} else if (newUrl !== pathname) {
router.push(newUrl);
}
}, [formStep, router, pathname, searchParams]);

// Update formStep when URL changes
useEffect(() => {
const stepFromUrl = parseInt(searchParams.get("step") || "0", 10);
if (stepFromUrl !== formStep) {
Expand Down
35 changes: 35 additions & 0 deletions web/src/components/popup/PopupFromQuery.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { useEffect } from "react";

import { usePopup } from "../admin/connectors/Popup";
import { PopupSpec } from "../admin/connectors/Popup";
import { useRouter } from "next/navigation";

interface PopupMessages {
[key: string]: PopupSpec;
}

export const usePopupFromQuery = (messages: PopupMessages) => {
const router = useRouter();
const { popup, setPopup } = usePopup();
Copy link

Choose a reason for hiding this comment

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

style: Consider destructuring setPopup only, as popup is not used in this hook


useEffect(() => {
Copy link

Choose a reason for hiding this comment

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

logic: The effect has missing dependencies. Add router, messages, and setPopup to the dependency array

const searchParams = new URLSearchParams(window.location.search);
// Get the value for search param with key "message"
const messageValue = searchParams.get("message");
// Check if any key from messages object is present in search params
for (const key in messages) {
if (messageValue === key) {
const popupMessage = messages[key];
console.log("popupMessage", popupMessage);
Copy link

Choose a reason for hiding this comment

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

style: Remove console.log statement before merging to production

const newUrl = `${window.location.pathname}`;
router.replace(newUrl);
setPopup(popupMessage);

// Exit the loop after handling the first match
break;
}
}
}, []);

return { popup };
Copy link

Choose a reason for hiding this comment

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

style: The popup value is not used within the hook, consider not returning it

};
Loading