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.

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

Jump to bottom

[REFACTOR] History data fetching #1698

Merged
merged 6 commits into from
Nov 27, 2024
Merged

[REFACTOR] History data fetching #1698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1e11e18
adds use-get-history hook, and replaces data fetching for history usi…
aristidesstaffieri Nov 25, 2024
2b8d3aa
removes stray log
aristidesstaffieri Nov 25, 2024
8812240
move loading render branch out of main render to avoid ternary
aristidesstaffieri Nov 25, 2024
c1612f5
tweak hasEmptyHistory check to account for an empty operations list
aristidesstaffieri Nov 25, 2024
be39fcd
renames use get history hook file to match repo conventions
aristidesstaffieri Nov 25, 2024
68003d3
renames var to follow code convention
aristidesstaffieri Nov 25, 2024
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
85 changes: 85 additions & 0 deletions extension/src/helpers/hooks/useGetHistory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { useReducer } from "react";

import { getAccountHistory } from "@shared/api/internal";
import { NetworkDetails } from "@shared/constants/stellar";
import { ServerApi } from "stellar-sdk/lib/horizon";

enum RequestState {
IDLE = "IDLE",
LOADING = "LOADING",
SUCCESS = "SUCCESS",
ERROR = "ERROR",
}

interface SuccessState {
state: RequestState.SUCCESS;
data: ServerApi.OperationRecord[];
error: null;
}

interface ErrorState {
state: RequestState.ERROR;
data: null;
error: unknown;
}

interface IdleState {
state: RequestState.IDLE;
data: null;
error: null;
}

interface LoadingState {
state: RequestState.LOADING;
data: null;
error: null;
}

type State = IdleState | LoadingState | SuccessState | ErrorState;

type Action =
| { type: "FETCH_DATA_START" }
| { type: "FETCH_DATA_SUCCESS"; payload: SuccessState["data"] }
| { type: "FETCH_DATA_ERROR"; payload: ErrorState["error"] };

const initialState: IdleState = {
state: RequestState.IDLE,
data: null,
error: null,
};

const reducer = (state: State, action: Action): State => {
switch (action.type) {
case "FETCH_DATA_START":
return { state: RequestState.LOADING, error: null, data: null };
case "FETCH_DATA_SUCCESS":
return { error: null, state: RequestState.SUCCESS, data: action.payload };
case "FETCH_DATA_ERROR":
return { data: null, state: RequestState.ERROR, error: action.payload };
default:
return state;
}
};

function useGetHistory(publicKey: string, networkDetails: NetworkDetails) {
const [state, dispatch] = useReducer(reducer, initialState);

const fetchData = async () => {
dispatch({ type: "FETCH_DATA_START" });
try {
const data = await getAccountHistory(publicKey, networkDetails);
dispatch({ type: "FETCH_DATA_SUCCESS", payload: data });
return data;
} catch (error) {
dispatch({ type: "FETCH_DATA_ERROR", payload: error });
return error;
}
};

return {
state,
fetchData,
};
}

export { useGetHistory, RequestState };
171 changes: 81 additions & 90 deletions extension/src/popup/views/AccountHistory/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import React, { useEffect, useState } from "react";
import { useSelector, useDispatch } from "react-redux";
import { useSelector } from "react-redux";
import { useTranslation } from "react-i18next";
import { Loader } from "@stellar/design-system";
import { Horizon } from "stellar-sdk";
import BigNumber from "bignumber.js";

import { getAccountHistory } from "@shared/api/internal";
import { ActionStatus } from "@shared/api/types";
import { SorobanTokenInterface } from "@shared/constants/soroban/token";

import { publicKeySelector } from "popup/ducks/accountServices";
Expand All @@ -33,8 +30,10 @@ import {
TransactionDetailProps,
} from "popup/components/accountHistory/TransactionDetail";
import { View } from "popup/basics/layout/View";
import { RequestState, useGetHistory } from "helpers/hooks/useGetHistory";

import "./styles.scss";
import { Loading } from "popup/components/Loading";

enum SELECTOR_OPTIONS {
ALL = "ALL",
Expand All @@ -55,13 +54,14 @@ export const AccountHistory = () => {
| null;

const { t } = useTranslation();
const dispatch = useDispatch();
const publicKey = useSelector(publicKeySelector);
const networkDetails = useSelector(settingsNetworkDetailsSelector);
const { accountBalances, accountBalanceStatus } = useSelector(
transactionSubmissionSelector,
);
const { accountBalances } = useSelector(transactionSubmissionSelector);
const { isHideDustEnabled } = useSelector(settingsSelector);
const { state: getHistoryState, fetchData } = useGetHistory(
publicKey,
networkDetails,
);

const [selectedSegment, setSelectedSegment] = useState(SELECTOR_OPTIONS.ALL);
const [historySegments, setHistorySegments] = useState(
Expand All @@ -76,14 +76,16 @@ export const AccountHistory = () => {
const [detailViewProps, setDetailViewProps] = useState(
defaultDetailViewProps,
);
const [isLoading, setIsLoading] = useState(false);

const stellarExpertUrl = getStellarExpertUrl(networkDetails);

const isAccountHistoryLoading =
historySegments === null ||
accountBalanceStatus === ActionStatus.IDLE ||
accountBalanceStatus === ActionStatus.PENDING;
useEffect(() => {
const getData = async () => {
await fetchData();
};
getData();
// eslint-disable-next-line react-hooks/exhaustive-deps
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we use fetchData as a dep of the useEffect it triggers another load of the data. React officially recommends using an empty array to run effects only on mount.

I think we should remove this lint rule but if we really don't want to we can also use the loading state to step around the extra call, but we won't be able to avoid the extra render.

Copy link
Contributor

@CassioMG CassioMG Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm fine using the empty array as useEffect dependency and disabling the lint rule on a case-by-case manner, I think it makes sense for cases where we need to run the code only once on mount 👍

I'm on the fence about removing the lint rule from the project, it could be useful in other cases where it's important to add more deps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah good point, it has helped me spot missing deps before so maybe it's ok to just step around it for these cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed - let's keep the lint rule and just disable when we explicitly just want to run on mount

}, []);

useEffect(() => {
const createSegments = (
Expand Down Expand Up @@ -141,86 +143,75 @@ export const AccountHistory = () => {
return segments;
};

const fetchAccountHistory = async () => {
try {
const operations = await getAccountHistory(publicKey, networkDetails);
setHistorySegments(createSegments(operations));
} catch (e) {
console.error(e);
}
};

const getData = async () => {
setIsLoading(true);
await fetchAccountHistory();
setIsLoading(false);
};

getData();
}, [publicKey, networkDetails, dispatch, isHideDustEnabled]);

return isDetailViewShowing ? (
<TransactionDetail {...detailViewProps} />
) : (
<>
<View.Content>
<div className="AccountHistory" data-testid="AccountHistory">
{isLoading ? (
<div className="AccountHistory__loader">
<Loader size="2rem" />
if (getHistoryState.state === RequestState.SUCCESS) {
setHistorySegments(createSegments(getHistoryState.data));
}
}, [
getHistoryState.state,
getHistoryState.data,
publicKey,
networkDetails,
isHideDustEnabled,
]);

const isLoaderShowing =
getHistoryState.state === RequestState.IDLE ||
getHistoryState.state === RequestState.LOADING;

if (isDetailViewShowing) {
return <TransactionDetail {...detailViewProps} />;
}

if (isLoaderShowing) {
return <Loading />;
}

const hasEmptyHistory = !getHistoryState?.data?.length;

return (
<View.Content>
<div className="AccountHistory" data-testid="AccountHistory">
<header className="AccountHistory__header">{t("Transactions")}</header>
<div className="AccountHistory__selector">
{Object.values(SELECTOR_OPTIONS).map((option) => (
<div
key={option}
className={`AccountHistory__selector__item ${
option === selectedSegment
? "AccountHistory__selector__item--active"
: ""
}`}
onClick={() => setSelectedSegment(option)}
>
{t(option)}
</div>
) : (
<>
<header className="AccountHistory__header">
{t("Transactions")}
</header>
<div className="AccountHistory__selector">
{Object.values(SELECTOR_OPTIONS).map((option) => (
<div
key={option}
className={`AccountHistory__selector__item ${
option === selectedSegment
? "AccountHistory__selector__item--active"
: ""
}`}
onClick={() => setSelectedSegment(option)}
>
{t(option)}
</div>
))}
</div>
<div className="AccountHistory__list">
{historySegments?.[SELECTOR_OPTIONS[selectedSegment]].length ? (
<HistoryList>
<>
{historySegments[SELECTOR_OPTIONS[selectedSegment]].map(
(operation: HistoryItemOperation) => (
<HistoryItem
key={operation.id}
accountBalances={accountBalances}
operation={operation}
publicKey={publicKey}
url={stellarExpertUrl}
networkDetails={networkDetails}
setDetailViewProps={setDetailViewProps}
setIsDetailViewShowing={setIsDetailViewShowing}
/>
),
)}
</>
</HistoryList>
) : (
<div>
{isAccountHistoryLoading
? null
: t("No transactions to show")}
</div>
))}
</div>
<div className="AccountHistory__list">
{historySegments?.[SELECTOR_OPTIONS[selectedSegment]].length ? (
<HistoryList>
<>
{historySegments[SELECTOR_OPTIONS[selectedSegment]].map(
(operation: HistoryItemOperation) => (
<HistoryItem
key={operation.id}
accountBalances={accountBalances}
operation={operation}
publicKey={publicKey}
url={stellarExpertUrl}
networkDetails={networkDetails}
setDetailViewProps={setDetailViewProps}
setIsDetailViewShowing={setIsDetailViewShowing}
/>
),
)}
</div>
</>
</>
</HistoryList>
) : (
<div>{hasEmptyHistory ? t("No transactions to show") : null}</div>
)}
</div>
</View.Content>
</>
</div>
</View.Content>
);
};
13 changes: 0 additions & 13 deletions extension/src/popup/views/AccountHistory/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,4 @@
.AccountHistory {
&__loader {
height: 100%;
width: 100%;
z-index: calc(var(--back--button-z-index) + 1);
position: absolute;
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
top: 0;
left: 0;
}

&__header {
font-size: 1.25rem;
margin-bottom: 1.5rem;
Expand Down