Skip to content

Commit

Permalink
Bug Fix: Delete feature (Fixes three issues) (#3383)
Browse files Browse the repository at this point in the history
Fixes: #3267 
Fixes: #3306 
Fixes: #3378 

Regarding: #3267 
Some of the z-indices were manged by zIndex parameter, while other
components where managed by Portals. Somehow the two conflicted and
created a weird mix of zIndices. I have changed popover's portal into
simple zindex parameter and parents, sibligns, and cousins accordingly.

Regarding: #3306 
The redirection to deleted chat happened because one of its parent
elements were a Link component. Previously, upon pressing delete,
stopEvent() was called to temporarily pause redirection, and redirection
would resume after the delete process. I have changed the Link component
into normal button with onClick redirection, maintaining the
stopPropogation behavior.

Regarding: #3378 
The default behavior of AlertDialog would wrap the text. But somehow
using portal on it would restrict text-wrap behavior. The text-wrap
behavior came back after removal of portal while working on #3267 .

deleting from /chat/{id}
<img width="1466" alt="image"
src="https://github.com/LAION-AI/Open-Assistant/assets/61619422/7faa1895-7894-41c3-ab57-5489db942641">

deleting from /chat
<img width="1440" alt="image"
src="https://github.com/LAION-AI/Open-Assistant/assets/61619422/d7b4494f-02f4-4953-bcd3-bf1c25de3b8f">

toast on delete
<img width="368" alt="image"
src="https://github.com/LAION-AI/Open-Assistant/assets/61619422/6ed228f1-64d3-4169-9657-aaff4e97ce06">

---------

Co-authored-by: notmd <notmd1811@gmail.com>
  • Loading branch information
CertifiedJoon and notmd authored Jun 13, 2023
1 parent 470fbe9 commit 1f0113f
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 67 deletions.
111 changes: 56 additions & 55 deletions website/src/components/Chat/ChatListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,26 +147,26 @@ export const ChatListItem = ({
style={{ insetInlineEnd: `8px` }}
gap="1.5"
zIndex={1}
// we have to stop the event, otherwise it would cause a navigation and close the sidebar on mobile
onClick={stopEvent}
>
{!isEditing && (
<>
<EditChatButton onClick={setIsEditing.on} />
<HideChatButton chatId={chat.id} onHide={onHide} />
{/* we have to stop the event, otherwise it would cause a navigation and close the sidebar on mobile */}
<div onClick={stopEvent}>
<Menu>
<MenuButton>
<ChatListItemIconButton label={t("more_actions")} icon={MoreHorizontal} />
</MenuButton>
<Portal appendToParentPortal={false} containerRef={rootRef}>
{/* higher z-index so that it is displayed over the mobile sidebar */}
<MenuList zIndex="var(--chakra-zIndices-popover)">
<OptOutDataButton chatId={chat.id} />
<DeleteChatButton chatId={chat.id} onDelete={onDelete} />
</MenuList>
</Portal>
</Menu>
</div>

<Menu>
<MenuButton>
<ChatListItemIconButton label={t("more_actions")} icon={MoreHorizontal} />
</MenuButton>
<Portal appendToParentPortal={false} containerRef={rootRef}>
{/* higher z-index so that it is displayed over the mobile sidebar */}
<MenuList zIndex="var(--chakra-zIndices-popover)">
<OptOutDataButton chatId={chat.id} />
<DeleteChatButton chatId={chat.id} onDelete={onDelete} />
</MenuList>
</Portal>
</Menu>
</>
)}
</Flex>
Expand All @@ -193,33 +193,37 @@ const HideChatButton = ({ chatId, onHide }: { chatId: string; onHide?: (params:
return <ChatListItemIconButton label={t("hide")} icon={EyeOff} onClick={onClick} />;
};

const DeleteChatButton = ({
chatId,
onDelete,
}: {
chatId: string;
onDelete?: (params: { chatId: string }) => void;
}) => {
const DeleteChatButton = ({ chatId, onDelete }: { chatId: string; onDelete: (params: { chatId: string }) => void }) => {
const { isOpen, onOpen, onClose } = useDisclosure();
const cancelRef = useRef();
const { t } = useTranslation(["chat", "common"]);
const { trigger: triggerDelete } = useSWRMutation<any, any, any, { chat_id: string }>(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const { trigger: triggerDelete, isMutating: isDeleting } = useSWRMutation<any, any, any, { chat_id: string }>(
API_ROUTES.DELETE_CHAT(chatId),
del
);
const onDeleteCallback = useCallback(async () => {

const router = useRouter();
const handleDelete = useCallback(async () => {
await triggerDelete({ chat_id: chatId });
onDelete?.({ chatId });
}, [onDelete, triggerDelete, chatId]);
if (router.query.id === chatId) {
// push to /chat if we are on the deleted chat
router.push("/chat");
} else {
onDelete({ chatId });
onClose();
}
}, [triggerDelete, chatId, router, onDelete, onClose]);

const alert = (
<AlertDialog isOpen={isOpen} leastDestructiveRef={cancelRef} onClose={onClose}>
<AlertDialogOverlay>
<AlertDialogContent>
<AlertDialogContent my="100px" flex="column">
<AlertDialogHeader fontSize="lg" fontWeight="bold">
{t("delete_chat")}
<Text>{t("delete_chat")}</Text>
</AlertDialogHeader>
<AlertDialogBody>
<Text fontWeight="bold" py="2">
<AlertDialogBody wordBreak="break-word">
<Text fontWeight="bold" py="2" textAlign="left">
{t("delete_confirmation")}
</Text>
<Text py="2">{t("delete_confirmation_detail")}</Text>
Expand All @@ -228,14 +232,15 @@ const DeleteChatButton = ({
<Button ref={cancelRef} onClick={onClose}>
{t("common:cancel")}
</Button>
<Button colorScheme="red" onClick={onDeleteCallback} ml={3}>
<Button colorScheme="red" onClick={handleDelete} ml={3} isLoading={isDeleting}>
{t("common:delete")}
</Button>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialogOverlay>
</AlertDialog>
);

return (
<>
<MenuItem onClick={onOpen} icon={<Trash size={16} />}>
Expand Down Expand Up @@ -263,35 +268,31 @@ const OptOutDataButton = ({ chatId }: { chatId: string }) => {
});
}, [chatId, onClose, t, toast, updateChat]);

const alert = (
<AlertDialog isOpen={isOpen} leastDestructiveRef={cancelRef} onClose={onClose}>
<AlertDialogOverlay>
<AlertDialogContent>
<AlertDialogHeader fontSize="lg" fontWeight="bold">
{t("chat:opt_out.dialog.title")}
</AlertDialogHeader>
<AlertDialogBody>
<Text py="2">{t("chat:opt_out.dialog.description")}</Text>
</AlertDialogBody>
<AlertDialogFooter>
<Button ref={cancelRef} onClick={onClose}>
{t("common:cancel")}
</Button>
<Button colorScheme="red" onClick={handleOptOut} ml={3} isLoading={isUpdating}>
{t("common:confirm")}
</Button>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialogOverlay>
</AlertDialog>
);

return (
<>
<MenuItem onClick={onOpen} icon={<FolderX size={16} />}>
{t("chat:opt_out.button")}
</MenuItem>
{alert}
<AlertDialog isOpen={isOpen} leastDestructiveRef={cancelRef} onClose={onClose}>
<AlertDialogOverlay>
<AlertDialogContent>
<AlertDialogHeader fontSize="lg" fontWeight="bold">
{t("chat:opt_out.dialog.title")}
</AlertDialogHeader>
<AlertDialogBody>
<Text py="2">{t("chat:opt_out.dialog.description")}</Text>
</AlertDialogBody>
<AlertDialogFooter>
<Button ref={cancelRef} onClick={onClose}>
{t("common:cancel")}
</Button>
<Button colorScheme="red" onClick={handleOptOut} ml={3} isLoading={isUpdating}>
{t("common:confirm")}
</Button>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialogOverlay>
</AlertDialog>
</>
);
};
Expand Down
1 change: 1 addition & 0 deletions website/src/components/Messages/BaseMessageEntry.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Avatar, AvatarProps, Box, BoxProps, Flex, useColorModeValue } from "@ch
import { forwardRef, lazy, Suspense } from "react";
import { colors } from "src/styles/Theme/colors";
import { StrictOmit } from "ts-essentials";

import { PluginUsageDetails } from "./PluginUsageDetails";
const RenderedMarkdown = lazy(() => import("./RenderedMarkdown"));

Expand Down
2 changes: 1 addition & 1 deletion website/src/components/Messages/MessageTableEntry.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ import useSWRMutation from "swr/mutation";

import { useUndeleteMessage } from "../../hooks/message/useUndeleteMessage";
import { BaseMessageEntry } from "./BaseMessageEntry";
import { MessageCreateDate } from "./MessageCreateDate";
import { MessageInlineEmojiRow } from "./MessageInlineEmojiRow";
import { MessageSyntheticBadge } from "./MessageSyntheticBadge";
import { MessageCreateDate } from "./MessageCreateDate";

interface MessageTableEntryProps {
message: Message;
Expand Down
6 changes: 3 additions & 3 deletions website/src/components/Messages/RenderedMarkdown.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import "katex/dist/katex.min.css";

import {
Box,
Button,
Expand All @@ -17,13 +19,11 @@ import { useTranslation } from "next-i18next";
import { memo, MouseEvent, useMemo, useState } from "react";
import ReactMarkdown from "react-markdown";
import type { ReactMarkdownOptions } from "react-markdown/lib/react-markdown";
import rehypeKatex from "rehype-katex";
import remarkGfm from "remark-gfm";
import remarkMath from "remark-math";
import rehypeKatex from "rehype-katex";
import { RenderedCodeblock } from "src/components/Messages/RenderedCodeblock";

import "katex/dist/katex.min.css";

interface RenderedMarkdownProps {
markdown: string;
disallowedElements?: string[];
Expand Down
3 changes: 1 addition & 2 deletions website/src/pages/_app.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import "../styles/globals.css";
import "focus-visible";

import type { AppContext, AppProps } from "next/app";
import App from "next/app";
import type { AppProps } from "next/app";
import Head from "next/head";
import { SessionProvider } from "next-auth/react";
import { appWithTranslation, useTranslation } from "next-i18next";
Expand Down
3 changes: 1 addition & 2 deletions website/src/pages/account/delete.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Button, Divider, Flex, Grid, Icon, ListItem, Text, UnorderedList } from "@chakra-ui/react";
import { Button, Divider, Flex, ListItem, Text, UnorderedList } from "@chakra-ui/react";
import Head from "next/head";
import Link from "next/link";
import React, { useCallback } from "react";
export { getStaticProps } from "src/lib/defaultServerSideProps";
import { useRouter } from "next/router";
Expand Down
2 changes: 1 addition & 1 deletion website/src/pages/api/admin/users.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { withAnyRole } from "src/lib/auth";
import { getValidDisplayName } from "src/lib/display_name_validation";
import { createApiClient } from "src/lib/oasst_client_factory";
import prisma from "src/lib/prismadb";
import { FetchUsersParams } from "src/types/Users";
import { getValidDisplayName } from "src/lib/display_name_validation";

/**
* The number of users to fetch in a single request. Could later be a query parameter.
Expand Down
1 change: 0 additions & 1 deletion website/src/pages/api/chat/plugins.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { withoutRole } from "src/lib/auth";
import { createInferenceClient } from "src/lib/oasst_inference_client";
import { PluginEntry } from "src/types/Chat";

const handler = withoutRole("banned", async (req, res, token) => {
const client = createInferenceClient(token);
Expand Down
4 changes: 2 additions & 2 deletions website/src/styles/Theme/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { type ThemeConfig, extendTheme } from "@chakra-ui/react";
import { extendTheme, type ThemeConfig } from "@chakra-ui/react";
import { Styles } from "@chakra-ui/theme-tools";
import { Inter } from "next/font/google";
import { withProse } from "@nikolovlazar/chakra-ui-prose";
import { Inter } from "next/font/google";

import { colors } from "./colors";
import { badgeTheme } from "./components/Badge";
Expand Down

0 comments on commit 1f0113f

Please sign in to comment.