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

Feat: implement DeleteGroupDialog #248

Merged
merged 7 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
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

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import {
Button,
DialogBody,
DialogClose,
DialogContent,
DialogFooter,
DialogHeader,
DialogTitle,
} from '@/components/ui';

interface IDeleteGroupDialogContentProps {
/**
* If true, close the parent UI component, such as Dialog, DropdownMenu, Drawer, etc., on cancel button click.
*/
closeParentOnCancel?: boolean;
/**
* The function to close the parent UI component.
*/
onParentClose?: () => void;
/**
* The function to close this delete dialog.
*/
onDialogClose: () => void;
}

export const DeleteGroupDialogContent = ({
closeParentOnCancel = true,
onParentClose,
onDialogClose,
}: IDeleteGroupDialogContentProps) => {
/**
* Handle the cancel button click.
* If the parentClose is true, close the parent dialog together with this dialog.
* If the parentClose is false, close only this dialog.
* @returns void
*/
const handleCancel = () => {
closeParentOnCancel && onParentClose?.();
};

/**
* Handle the delete button click.
* If the DELETE request is successful, show a success message and close the dialog and drawer.
* If the DELETE request is failed, show an error message and close the dialog.
* @returns void
*/
const handleDelete = async () => {
alert('delete');
onDialogClose();
};

return (
<DialogContent>
<DialogHeader>
<DialogTitle>Delete Group</DialogTitle>
</DialogHeader>
<DialogBody>
<p>Are you sure you want to delete this group?</p>
</DialogBody>
<DialogFooter>
<DialogClose asChild>
<Button variant="cancel" size="sm" onClick={handleCancel}>
Cancel
</Button>
</DialogClose>
<Button variant="error" size="sm" onClick={handleDelete}>
Delete
</Button>
</DialogFooter>
</DialogContent>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { DeleteIcon } from '@/assets/images/icons';
import {
DialogRoot,
DialogTrigger,
DropdownMenuButton,
DropdownMenuButtonIcon,
DropdownMenuButtonText,
Icon,
} from '@/components/ui';

import { useState } from 'react';

import { DeleteGroupDialogContent } from './DeleteGroupDialogContent';

interface IGroupCardDropdownMenuDeleteButtonProps {
/**
* The function to close the dropdown menu.
*/
onDropdownMenuClose: () => void;
}

export const DeleteGroupDialogTriggerButton = ({
onDropdownMenuClose,
}: IGroupCardDropdownMenuDeleteButtonProps) => {
const [isDialogOpen, setIsDialogOpen] = useState(false);

return (
<DialogRoot open={isDialogOpen} onOpenChange={setIsDialogOpen}>
<DialogTrigger asChild>
<DropdownMenuButton>
<DropdownMenuButtonIcon>
<Icon icon={DeleteIcon} size={5} color="danger" />
</DropdownMenuButtonIcon>
<DropdownMenuButtonText>Delete</DropdownMenuButtonText>
</DropdownMenuButton>
</DialogTrigger>
<DeleteGroupDialogContent
onParentClose={onDropdownMenuClose}
onDialogClose={() => setIsDialogOpen(false)}
/>
</DialogRoot>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { PenIcon } from '@/assets/images/icons';
import {
DropdownMenuButton,
DropdownMenuButtonIcon,
DropdownMenuButtonText,
DropdownMenuContent,
DropdownMenuItem,
Icon,
} from '@/components/ui';

import { DeleteGroupDialogTriggerButton } from './DeleteGroupDialogTriggerButton';

interface IGroupCardDropdownMenuContentProps {
/**
* Function to handle the rename button click.
*/
handleRenameClick: () => void;

/**
* The function to close the dropdown menu.
*/
onDropdownMenuClose: () => void;
}

export const GroupCardDropdownMenuContent = ({
handleRenameClick,
onDropdownMenuClose,
}: IGroupCardDropdownMenuContentProps) => {
/**
* The function that is called when the dropdown menu item is selected.
* e.preventDefault() prevents the dropdown menu from closing when selecting that item.
* This is necessary because closing dropdown menu also closes the dialog unintentionally.
* @see {@link https://www.radix-ui.com/primitives/docs/components/dropdown-menu#item}
*
* @param e - The event object
* @returns void
*/
const handleSelect = (e: Event) => {
e.preventDefault();
};

return (
<DropdownMenuContent>
<DropdownMenuItem>
<DropdownMenuButton onClick={handleRenameClick}>
<DropdownMenuButtonIcon>
<Icon icon={PenIcon} size={5} color="primary" />
</DropdownMenuButtonIcon>
<DropdownMenuButtonText>Rename</DropdownMenuButtonText>
</DropdownMenuButton>
</DropdownMenuItem>
<DropdownMenuItem onSelect={handleSelect}>
<DeleteGroupDialogTriggerButton onDropdownMenuClose={onDropdownMenuClose} />
</DropdownMenuItem>
</DropdownMenuContent>
);
};
Copy link
Member Author

@kanta1207 kanta1207 Feb 27, 2024

Choose a reason for hiding this comment

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

This file has changed just because I removed FC type.

I did the change only to this file for now because the file was in this PR's scope. Maybe I'll refactor rest of FC types in other files later in different scope.

Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,29 @@
import { MenuKebabIcon } from '@/assets/images/icons';
import { Button, DropdownMenu, DropdownMenuTrigger, Icon } from '@/components/ui';

import { FC } from 'react';
import { useState } from 'react';

import { GroupCardDropdownMenuContent } from './GroupCardDropdownMenuContent';

interface IGroupCardMenuButtonProps {
handleRenameClick: () => void;
}

export const GroupCardDropdownMenuTriggerButton: FC<IGroupCardMenuButtonProps> = ({
export const GroupCardDropdownMenuTriggerButton = ({
handleRenameClick,
}) => {
}: IGroupCardMenuButtonProps) => {
const [isDropdownMenuOpen, setIsDropdownMenuOpen] = useState(false);
return (
<DropdownMenu>
<DropdownMenu open={isDropdownMenuOpen} onOpenChange={setIsDropdownMenuOpen}>
<DropdownMenuTrigger asChild>
<Button variant="ghost" className="w-12">
<Icon icon={MenuKebabIcon} size={4.5} />
</Button>
</DropdownMenuTrigger>
<GroupCardDropdownMenuContent handleRenameClick={handleRenameClick} />
Copy link
Member Author

Choose a reason for hiding this comment

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

Because I separated GroupCardDropdownMenuContent from trigger as a different component,
I ended up having to pass onDropdownMenuClose prop from this file, and it's props drilling among 4 layers.

(GroupCardDropdownMenuTriggerButton => GroupCardDropdownMenuContent => DeleteGroupDialogTriggerButton => DeleteGroupDialogContent)

The directory structure itself is reasonable one tho, let me know if you feel we have to do something for this props drilling. I personally chose to keep this directory structure for now.

Copy link
Member

@nick-y-ito nick-y-ito Feb 27, 2024

Choose a reason for hiding this comment

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

I personally chose to keep this directory structure for now.

I agree. I think it's acceptable for now.

<GroupCardDropdownMenuContent
handleRenameClick={handleRenameClick}
onDropdownMenuClose={() => setIsDropdownMenuOpen(false)}
/>
</DropdownMenu>
);
};
2 changes: 1 addition & 1 deletion src/lib/api/group/client/groupApiClient.client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export const deleteMember = async (
): Promise<Result<undefined, string>> => {
try {
await request({
url: `${BACKEND_API_DOMAIN}/groups/${groupId}/users/${userId}`,
url: `${API_BASE_URL}/groups/${groupId}/users/${userId}`,
Copy link
Member Author

Choose a reason for hiding this comment

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

Found out thisBACKEND_API_DOMAIN variable (which doesn't exist) is set as base url in here, and caused build error. The functionality shouldn't be also working tho, don't know how it's in develop branch now...

method: 'DELETE',
});
return Ok(undefined);
Expand Down
Loading