From 1f6450ea03e7bd4bd41c6801376b08c181db15df Mon Sep 17 00:00:00 2001 From: Ryo Narita Date: Thu, 17 Dec 2020 18:38:52 +0900 Subject: [PATCH] Refactor deployment list page --- pkg/app/web/src/modules/deployments.test.ts | 27 ++-- pkg/app/web/src/modules/deployments.ts | 19 ++- pkg/app/web/src/pages/deployments/index.tsx | 165 ++++++++------------ pkg/app/web/src/types/module.ts | 1 + 4 files changed, 88 insertions(+), 124 deletions(-) create mode 100644 pkg/app/web/src/types/module.ts diff --git a/pkg/app/web/src/modules/deployments.test.ts b/pkg/app/web/src/modules/deployments.test.ts index c08f12f41a..73c017e7bf 100644 --- a/pkg/app/web/src/modules/deployments.test.ts +++ b/pkg/app/web/src/modules/deployments.test.ts @@ -1,3 +1,4 @@ +import { LoadingStatus } from "../types/module"; import { dummyDeployment } from "../__fixtures__/dummy-deployment"; import { CommandModel, fetchCommand, CommandStatus } from "./commands"; import { @@ -17,8 +18,7 @@ const initialState = { entities: {}, hasMore: true, ids: [], - isLoadingItems: false, - isLoadingMoreItems: false, + status: "idle" as LoadingStatus, loading: {}, }; @@ -70,8 +70,7 @@ describe("deploymentsSlice reducer", () => { entities: {}, hasMore: true, ids: [], - isLoadingItems: false, - isLoadingMoreItems: false, + status: "idle", loading: { "deployment-1": true, }, @@ -138,7 +137,7 @@ describe("deploymentsSlice reducer", () => { }) ).toEqual({ ...initialState, - isLoadingItems: true, + status: "loading", }); }); @@ -147,7 +146,7 @@ describe("deploymentsSlice reducer", () => { deploymentsSlice.reducer( { ...initialState, - isLoadingItems: true, + status: "loading", }, { type: fetchDeployments.rejected.type, @@ -155,7 +154,7 @@ describe("deploymentsSlice reducer", () => { ) ).toEqual({ ...initialState, - isLoadingItems: false, + status: "failed", }); }); @@ -164,7 +163,7 @@ describe("deploymentsSlice reducer", () => { deploymentsSlice.reducer( { ...initialState, - isLoadingItems: true, + status: "loading", }, { type: fetchDeployments.fulfilled.type, @@ -176,7 +175,7 @@ describe("deploymentsSlice reducer", () => { entities: { [dummyDeployment.id]: dummyDeployment }, hasMore: false, ids: [dummyDeployment.id], - isLoadingItems: false, + status: "succeeded", }); }); }); @@ -187,24 +186,24 @@ describe("deploymentsSlice reducer", () => { deploymentsSlice.reducer(initialState, { type: fetchMoreDeployments.pending.type, }) - ).toEqual({ ...initialState, isLoadingMoreItems: true }); + ).toEqual({ ...initialState, status: "loading" }); }); it(`should handle ${fetchMoreDeployments.rejected.type}`, () => { expect( deploymentsSlice.reducer( - { ...initialState, isLoadingMoreItems: true }, + { ...initialState, status: "loading" }, { type: fetchMoreDeployments.rejected.type, } ) - ).toEqual({ ...initialState, isLoadingMoreItems: false }); + ).toEqual({ ...initialState, status: "failed" }); }); it(`should handle ${fetchMoreDeployments.fulfilled.type}`, () => { expect( deploymentsSlice.reducer( - { ...initialState, isLoadingMoreItems: true }, + { ...initialState, status: "loading" }, { type: fetchMoreDeployments.fulfilled.type, payload: [dummyDeployment], @@ -212,10 +211,10 @@ describe("deploymentsSlice reducer", () => { ) ).toEqual({ ...initialState, - isLoadingMoreItems: false, hasMore: false, ids: [dummyDeployment.id], entities: { [dummyDeployment.id]: dummyDeployment }, + status: "succeeded", }); }); }); diff --git a/pkg/app/web/src/modules/deployments.ts b/pkg/app/web/src/modules/deployments.ts index f2ef2d77c4..7c5b9d2059 100644 --- a/pkg/app/web/src/modules/deployments.ts +++ b/pkg/app/web/src/modules/deployments.ts @@ -12,6 +12,7 @@ import { import * as deploymentsApi from "../api/deployments"; import { fetchCommand, CommandModel, CommandStatus } from "./commands"; import { AppState } from "."; +import { LoadingStatus } from "../types/module"; export type Deployment = Required; export type Stage = Required; @@ -64,16 +65,14 @@ export const { } = deploymentsAdapter.getSelectors(); const initialState = deploymentsAdapter.getInitialState<{ + status: LoadingStatus; loading: Record; canceling: Record; - isLoadingItems: boolean; - isLoadingMoreItems: boolean; hasMore: boolean; }>({ + status: "idle", loading: {}, canceling: {}, - isLoadingItems: false, - isLoadingMoreItems: false, hasMore: true, }); @@ -187,34 +186,34 @@ export const deploymentsSlice = createSlice({ state.loading[action.meta.arg] = false; }) .addCase(fetchDeployments.pending, (state) => { - state.isLoadingItems = true; + state.status = "loading"; state.hasMore = true; }) .addCase(fetchDeployments.fulfilled, (state, action) => { + state.status = "succeeded"; deploymentsAdapter.removeAll(state); if (action.payload.length > 0) { deploymentsAdapter.upsertMany(state, action.payload); } - state.isLoadingItems = false; if (action.payload.length < ITEMS_PER_PAGE) { state.hasMore = false; } }) .addCase(fetchDeployments.rejected, (state) => { - state.isLoadingItems = false; + state.status = "failed"; }) .addCase(fetchMoreDeployments.pending, (state) => { - state.isLoadingMoreItems = true; + state.status = "loading"; }) .addCase(fetchMoreDeployments.fulfilled, (state, action) => { + state.status = "succeeded"; deploymentsAdapter.upsertMany(state, action.payload); - state.isLoadingMoreItems = false; if (action.payload.length < FETCH_MORE_ITEMS_PER_PAGE) { state.hasMore = false; } }) .addCase(fetchMoreDeployments.rejected, (state) => { - state.isLoadingMoreItems = false; + state.status = "failed"; }) .addCase(cancelDeployment.pending, (state, action) => { diff --git a/pkg/app/web/src/pages/deployments/index.tsx b/pkg/app/web/src/pages/deployments/index.tsx index b4ced02aa4..ad4f3d6f69 100644 --- a/pkg/app/web/src/pages/deployments/index.tsx +++ b/pkg/app/web/src/pages/deployments/index.tsx @@ -1,4 +1,5 @@ import { + Box, Button, CircularProgress, Divider, @@ -32,22 +33,11 @@ import { fetchMoreDeployments, } from "../../modules/deployments"; import { useInView } from "react-intersection-observer"; +import { LoadingStatus } from "../../types/module"; +import { UI_TEXT_REFRESH } from "../../constants/ui-text"; +import { useStyles as useButtonStyles } from "../../styles/button"; const useStyles = makeStyles((theme) => ({ - root: { - display: "flex", - overflow: "hidden", - flex: 1, - flexDirection: "column", - }, - main: { - display: "flex", - overflow: "hidden", - flex: 1, - }, - toolbarSpacer: { - flexGrow: 1, - }, deploymentLists: { listStyle: "none", padding: theme.spacing(3), @@ -56,24 +46,10 @@ const useStyles = makeStyles((theme) => ({ flex: 1, overflowY: "scroll", }, - loadingContainer: { - display: "flex", - justifyContent: "center", - marginTop: theme.spacing(3), - }, - deployments: { - listStyle: "none", - padding: 0, - }, date: { marginTop: theme.spacing(2), marginBottom: theme.spacing(2), }, - empty: { - display: "flex", - justifyContent: "center", - marginTop: theme.spacing(3), - }, })); const sortComp = (a: string | number, b: string | number): number => { @@ -85,26 +61,20 @@ function filterUndefined(value: TValue | undefined): value is TValue { } const useGroupedDeployments = (): [ - boolean, - boolean, + LoadingStatus, boolean, Record ] => { - const [ - isLoadingItems, - isLoadingMoreItems, - hasMore, - deployments, - ] = useSelector( - (state) => [ - state.deployments.isLoadingItems, - state.deployments.isLoadingMoreItems, - state.deployments.hasMore, - selectDeploymentIds(state.deployments) - .map((id) => selectDeploymentById(state.deployments, id)) - .filter(filterUndefined), - ] - ); + const [status, hasMore, deployments] = useSelector< + AppState, + [LoadingStatus, boolean, Deployment[]] + >((state) => [ + state.deployments.status, + state.deployments.hasMore, + selectDeploymentIds(state.deployments) + .map((id) => selectDeploymentById(state.deployments, id)) + .filter(filterUndefined), + ]); const result: Record = {}; @@ -116,60 +86,57 @@ const useGroupedDeployments = (): [ result[dateStr].push(deployment); }); - return [isLoadingItems, isLoadingMoreItems, hasMore, result]; + return [status, hasMore, result]; }; export const DeploymentIndexPage: FC = memo(function DeploymentIndexPage() { const classes = useStyles(); + const buttonClasses = useButtonStyles(); const dispatch = useDispatch(); const listRef = useRef(null); - const [ - isLoading, - isLoadingMoreItems, - hasMore, - groupedDeployments, - ] = useGroupedDeployments(); + const [status, hasMore, groupedDeployments] = useGroupedDeployments(); const [isOpenFilter, setIsOpenFilter] = useState(false); const [ref, inView] = useInView({ rootMargin: "400px", root: listRef.current, }); + const isLoading = status === "loading"; useEffect(() => { dispatch(fetchApplications()); + dispatch(fetchDeployments()); }, [dispatch]); useEffect(() => { - if ( - inView && - hasMore && - isLoading === false && - isLoadingMoreItems === false - ) { + if (inView && hasMore && isLoading) { dispatch(fetchMoreDeployments()); } - }, [dispatch, inView, hasMore, isLoading, isLoadingMoreItems]); + }, [dispatch, inView, hasMore, isLoading]); - const handleChangeFilter = useCallback(() => { + const handleFilterChange = useCallback(() => { dispatch(fetchDeployments()); }, [dispatch]); - const handleRefresh = (): void => { + const handleRefreshClick = useCallback(() => { dispatch(fetchDeployments()); - }; + }, [dispatch]); const dates = Object.keys(groupedDeployments).sort(sortComp); return ( -
+ -
+
+ + + ); }); diff --git a/pkg/app/web/src/types/module.ts b/pkg/app/web/src/types/module.ts new file mode 100644 index 0000000000..4d4d2312a2 --- /dev/null +++ b/pkg/app/web/src/types/module.ts @@ -0,0 +1 @@ +export type LoadingStatus = "idle" | "loading" | "succeeded" | "failed";