Skip to content

Refactor piped list components #1446

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

Merged
pipecd-bot merged 1 commit into from
Jan 19, 2021
Merged

Refactor piped list components #1446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions pkg/app/web/src/constants/ui-text.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ export const UI_TEXT_SAVE = "Save";
export const UI_TEXT_CANCEL = "Cancel";
export const UI_TEXT_ADD = "Add";
export const UI_TEXT_EDIT = "Edit";
export const UI_TEXT_DISABLE = "Disable";
export const UI_TEXT_ENABLE = "Enable";
export const UI_TEXT_RECREATE_KEY = "Recreate Key";
export const UI_TEXT_REFRESH = "REFRESH";
export const UI_TEXT_CLOSE = "Close";
export const UI_TEXT_DELETE = "Delete";
Expand Down
169 changes: 47 additions & 122 deletions pkg/app/web/src/pages/settings/piped.tsx → ...pp/web/src/pages/settings/piped/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import {
DialogContent,
DialogTitle,
Divider,
IconButton,
makeStyles,
Menu,
MenuItem,
Paper,
Table,
TableBody,
Expand All @@ -18,23 +15,24 @@ import {
TableRow,
TextField,
Toolbar,
Typography,
} from "@material-ui/core";
import {
Add as AddIcon,
Close as CloseIcon,
MoreVert as MoreVertIcon,
FilterList as FilterIcon,
} from "@material-ui/icons";
import clsx from "clsx";
import dayjs from "dayjs";
import React, { FC, memo, useCallback, useState } from "react";
import { useDispatch, useSelector } from "react-redux";
import { AddPipedDrawer } from "../../components/add-piped-drawer";
import { EditPipedDrawer } from "../../components/edit-piped-drawer";
import { PipedFilter, FilterValues } from "../../components/piped-filter";
import { UI_TEXT_FILTER, UI_TEXT_HIDE_FILTER } from "../../constants/ui-text";
import { AppState } from "../../modules";
import { AddPipedDrawer } from "../../../components/add-piped-drawer";
import { EditPipedDrawer } from "../../../components/edit-piped-drawer";
import { FilterValues, PipedFilter } from "../../../components/piped-filter";
import {
UI_TEXT_ADD,
UI_TEXT_CLOSE,
UI_TEXT_FILTER,
UI_TEXT_HIDE_FILTER,
} from "../../../constants/ui-text";
import { AppState } from "../../../modules";
import {
clearRegisteredPipedInfo,
disablePiped,
Expand All @@ -44,20 +42,16 @@ import {
recreatePipedKey,
RegisteredPiped,
selectAll,
} from "../../modules/pipeds";
import { AppDispatch } from "../../store";
} from "../../../modules/pipeds";
import { AppDispatch } from "../../../store";
import { PipedTableRow } from "./piped-table-row";

const useStyles = makeStyles((theme) => ({
disabledItem: {
background: theme.palette.grey[200],
},
const useStyles = makeStyles(() => ({
toolbarSpacer: {
flexGrow: 1,
},
}));

const ITEM_HEIGHT = 48;

const usePipeds = (filterValues: FilterValues): Piped[] => {
const pipeds = useSelector<AppState, Piped[]>((state) =>
selectAll(state.pipeds)
Expand All @@ -78,47 +72,44 @@ export const SettingsPipedPage: FC = memo(function SettingsPipedPage() {
const classes = useStyles();
const [openFilter, setOpenFilter] = useState(false);
const [isOpenForm, setIsOpenForm] = useState(false);
const [actionTarget, setActionTarget] = useState<Piped | null>(null);
const [editPipedId, setEditPipedId] = useState<string | null>(null);
const [filterValues, setFilterValues] = useState<FilterValues>({
enabled: true,
});
const [anchorEl, setAnchorEl] = useState<HTMLButtonElement | null>(null);
const isOpenMenu = Boolean(anchorEl);
const dispatch = useDispatch<AppDispatch>();
const pipeds = usePipeds(filterValues);

const registeredPiped = useSelector<AppState, RegisteredPiped | null>(
(state) => state.pipeds.registeredPiped
);

const handleMenuOpen = useCallback(
(event: React.MouseEvent<HTMLButtonElement>, piped: Piped): void => {
setActionTarget(piped);
setAnchorEl(event.currentTarget);
const handleDisable = useCallback(
(id: string) => {
dispatch(disablePiped({ pipedId: id })).then(() => {
dispatch(fetchPipeds(true));
});
},
[]
[dispatch]
);
const handleEnable = useCallback(
(id: string) => {
dispatch(enablePiped({ pipedId: id })).then(() => {
dispatch(fetchPipeds(true));
});
},
[dispatch]
);

const closeMenu = useCallback(() => {
setAnchorEl(null);
setTimeout(() => {
setActionTarget(null);
}, 200);
}, []);

const handleDisableClick = useCallback(() => {
closeMenu();
if (!actionTarget) {
return;
}

const act = actionTarget.disabled ? enablePiped : disablePiped;
const handleRecreate = useCallback(
(id: string) => {
dispatch(recreatePipedKey({ pipedId: id }));
},
[dispatch]
);

dispatch(act({ pipedId: actionTarget.id })).then(() => {
dispatch(fetchPipeds(true));
});
}, [dispatch, actionTarget, closeMenu]);
const handleEdit = useCallback((id: string) => {
setEditPipedId(id);
}, []);

const handleClose = useCallback(() => {
setIsOpenForm(false);
Expand All @@ -129,20 +120,6 @@ export const SettingsPipedPage: FC = memo(function SettingsPipedPage() {
dispatch(fetchPipeds(true));
}, [dispatch]);

const handleRecreate = useCallback(() => {
if (actionTarget) {
dispatch(recreatePipedKey({ pipedId: actionTarget.id }));
}
closeMenu();
}, [dispatch, actionTarget, closeMenu]);

const handleEdit = useCallback(() => {
if (actionTarget) {
setEditPipedId(actionTarget.id);
}
closeMenu();
}, [actionTarget, closeMenu]);

const handleEditClose = useCallback(() => {
setEditPipedId(null);
}, []);
Expand All @@ -155,7 +132,7 @@ export const SettingsPipedPage: FC = memo(function SettingsPipedPage() {
startIcon={<AddIcon />}
onClick={() => setIsOpenForm(true)}
>
ADD
{UI_TEXT_ADD}
</Button>
<div className={classes.toolbarSpacer} />
<Button
Expand All @@ -182,36 +159,14 @@ export const SettingsPipedPage: FC = memo(function SettingsPipedPage() {
</TableHead>
<TableBody>
{pipeds.map((piped) => (
<TableRow
key={`pipe-${piped.id}`}
className={clsx({ [classes.disabledItem]: piped.disabled })}
>
<TableCell>
<Typography variant="subtitle2">
{`${piped.name} (${piped.id.slice(0, 8)})`}
</Typography>
</TableCell>
<TableCell>{piped.version}</TableCell>
<TableCell>
<Typography variant="body2" color="textSecondary">
{piped.desc}
</Typography>
</TableCell>
<TableCell>
{piped.startedAt === 0
? "Not Yet Started"
: dayjs(piped.startedAt * 1000).fromNow()}
</TableCell>
<TableCell align="right">
<IconButton
edge="end"
aria-label="open menu"
onClick={(e) => handleMenuOpen(e, piped)}
>
<MoreVertIcon />
</IconButton>
</TableCell>
</TableRow>
<PipedTableRow

@khanhtc1202 khanhtc1202 Jan 19, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nits, prefix Piped is redundant to me 👀

@cakecatz cakecatz Jan 19, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@khanhtc1202
If there were no Piped, wouldn't it be hard to distinguish it from the TableRow in material-ui?

@khanhtc1202 khanhtc1202 Jan 19, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ops, I see 👀 In that case, maybe my suggestion is: rename the component to TableRow and in the calling side

import { TableRow as PipedTableRow } from "./piped-table-row";

but in the second thought, It is quite laborious to do so, so the current implementation is LGTM 👍
I'm going to merge this PR 🏃‍♂️

key={piped.id}
pipedId={piped.id}
onEdit={handleEdit}
onRecreateKey={handleRecreate}
onDisable={handleDisable}
onEnable={handleEnable}
/>
))}
</TableBody>
</Table>
Expand All @@ -222,36 +177,6 @@ export const SettingsPipedPage: FC = memo(function SettingsPipedPage() {
)}
</Box>

<Menu
id="piped-menu"
anchorEl={anchorEl}
keepMounted
open={isOpenMenu}
onClose={() => closeMenu()}
PaperProps={{
style: {
maxHeight: ITEM_HEIGHT * 4.5,
width: "20ch",
},
}}
>
{actionTarget && actionTarget.disabled ? (
<MenuItem onClick={handleDisableClick}>Enable</MenuItem>
) : (
[
<MenuItem key="piped-menu-edit" onClick={handleEdit}>
Edit
</MenuItem>,
<MenuItem key="piped-menu-recreate" onClick={handleRecreate}>
Recreate Key
</MenuItem>,
<MenuItem key="piped-menu-disable" onClick={handleDisableClick}>
Disable
</MenuItem>,
]
)}
</Menu>

<AddPipedDrawer open={isOpenForm} onClose={handleClose} />
<EditPipedDrawer pipedId={editPipedId} onClose={handleEditClose} />

Expand All @@ -274,7 +199,7 @@ export const SettingsPipedPage: FC = memo(function SettingsPipedPage() {
/>
<Box display="flex" justifyContent="flex-end" m={1} mt={2}>
<Button color="primary" onClick={handleClosePipedInfo}>
CLOSE
{UI_TEXT_CLOSE}
</Button>
</Box>
</DialogContent>
Expand Down
Loading