[REFACTOR] History data fetching #1698
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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) { | ||
|
There was a problem hiding this comment. thanks for implementing this! There was a problem hiding this comment. It's a little different than the example we discussed in the issue but this seemed easy to work with, open to any feedback on this pattern! |
||
| 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 }; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,9 @@ | ||
| import React, { useEffect, useState } from "react"; | ||
| import { useSelector } from "react-redux"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { Horizon } from "stellar-sdk"; | ||
| import BigNumber from "bignumber.js"; | ||
|
|
||
| import { SorobanTokenInterface } from "@shared/constants/soroban/token"; | ||
|
|
||
| import { publicKeySelector } from "popup/ducks/accountServices"; | ||
|
|
@@ -33,8 +30,10 @@ import { | |
| TransactionDetailProps, | ||
| } from "popup/components/accountHistory/TransactionDetail"; | ||
| import { View } from "popup/basics/layout/View"; | ||
| import { RequestState, useGetHistory } from "helpers/hooks/use-get-history"; | ||
|
|
||
| import "./styles.scss"; | ||
| import { Loading } from "popup/components/Loading"; | ||
|
|
||
| enum SELECTOR_OPTIONS { | ||
| ALL = "ALL", | ||
|
|
@@ -55,13 +54,14 @@ export const AccountHistory = () => { | |
| | null; | ||
|
|
||
| const { t } = useTranslation(); | ||
| const publicKey = useSelector(publicKeySelector); | ||
| const networkDetails = useSelector(settingsNetworkDetailsSelector); | ||
| 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( | ||
|
|
@@ -76,14 +76,16 @@ export const AccountHistory = () => { | |
| const [detailViewProps, setDetailViewProps] = useState( | ||
| defaultDetailViewProps, | ||
| ); | ||
|
|
||
| const stellarExpertUrl = getStellarExpertUrl(networkDetails); | ||
|
|
||
| useEffect(() => { | ||
| const getData = async () => { | ||
| await fetchData(); | ||
| }; | ||
| getData(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
|
There was a problem hiding this comment. If we use 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. There was a problem hiding this comment. 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 There was a problem hiding this comment. yeah good point, it has helped me spot missing deps before so maybe it's ok to just step around it for these cases? There was a problem hiding this comment. Agreed - let's keep the lint rule and just disable when we explicitly just want to run on mount |
||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| const createSegments = ( | ||
|
|
@@ -141,86 +143,75 @@ export const AccountHistory = () => { | |
| return segments; | ||
| }; | ||
|
|
||
| if (getHistoryState.state === RequestState.SUCCESS) { | ||
| setHistorySegments(createSegments(getHistoryState.data)); | ||
| } | ||
| }, [ | ||
| getHistoryState.state, | ||
| getHistoryState.data, | ||
| publicKey, | ||
| networkDetails, | ||
| isHideDustEnabled, | ||
| ]); | ||
|
|
||
| const showLoader = | ||
|
There was a problem hiding this comment. NIT: let's call this There was a problem hiding this comment. yup, renamed in 68003d3 |
||
| getHistoryState.state === RequestState.IDLE || | ||
| getHistoryState.state === RequestState.LOADING; | ||
|
|
||
| if (isDetailViewShowing) { | ||
| return <TransactionDetail {...detailViewProps} />; | ||
| } | ||
|
|
||
| if (showLoader) { | ||
| 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> | ||
| ))} | ||
| </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>{hasEmptyHistory ? t("No transactions to show") : null}</div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </View.Content> | ||
| ); | ||
| }; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,4 @@ | ||
| .AccountHistory { | ||
| &__header { | ||
| font-size: 1.25rem; | ||
| margin-bottom: 1.5rem; | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: can you call this file useGetHistory.tsx to conform to the naming convention in the rest of the repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops old habit, renamed in be39fcd