-
Notifications
You must be signed in to change notification settings - Fork 0
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
added transaction tracking #51
Conversation
export interface TrackedTransactionsSliceType { | ||
[sessionId: string]: { | ||
transactions: SignedTransactionType[]; | ||
status?: TransactionBatchStatusesEnum; |
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.
TransactionBatchStatusesEnum | TransactionServerStatusesEnum
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.
done
@@ -55,6 +56,19 @@ export class TransactionManager { | |||
} | |||
}; | |||
|
|||
public track = async ( | |||
signedTransactions: Transaction[], | |||
options?: { enableToasts: boolean } |
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.
disableToasts boolean default undefined ?
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.
i change it to default {enableToasts:true}
|
||
export async function checkTransactionStatus( | ||
props: TransactionsTrackerType & { | ||
shouldRefreshBalance?: boolean; | ||
} | ||
) { | ||
const { pendingSessions } = getPendingStoreTransactions(); | ||
const { pendingTrackedSessions: pendingSessions } = |
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.
why not keep sessions, since we don't have any other sessions
const recheckStatus = () => { | ||
checkTransactionStatus({ | ||
shouldRefreshBalance: isWebsocketCompleted, | ||
getTransactionsByHash, | ||
...props | ||
getTransactionsByHash: getTransactionsByHashes |
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.
strange name difference. & why not use it as a default in checkTransactionStatus and allow optional override
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.
removed it and used as default in checkTransactionStatus
) => { | ||
getStore().setState(({ toasts: state }) => { | ||
const toastId = | ||
currentToastId || `custom-toast-${state.customToasts.length + 1}`; |
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.
add toast 0 --> custom-toast-1
add toast 1 --> custom-toast-2
delete toast 0
add toast --> custom-toast-2 => you will have duplicates of custom-toast-2
solution state.customToasts --> get last toast index and increase it by one
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.
done
type: ToastsEnum.transaction, | ||
startTimestamp: getUnixTimestamp(), | ||
toastId: | ||
toastId || `transaction-toast-${state.transactionToasts.length + 1}` |
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.
check vulenrability here if there is one
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.
i did the same thing as in custom case
@@ -0,0 +1,11 @@ | |||
import { StoreType } from 'store/store.types'; | |||
|
|||
export const networkSliceSelector = ({ network }: StoreType) => network; |
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.
duplicate
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.
removed
import { TransactionServerStatusesEnum } from 'types/enums.types'; | ||
import { SignedTransactionType } from 'types/transactions.types'; | ||
|
||
export const transactionsSliceSelector = ({ trackedTransactions }: StoreType) => |
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.
trackedTransactionsSliceSelector
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.
done
* | ||
* **⚠️ Warning**: Toasts with components will not be persisted on page reload because React components are not serializable | ||
*/ | ||
component: (() => JSX.Element) | null; |
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.
is this used? since we are no longer in React
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.
i removed for now all the custom types and let only the generic one
@@ -0,0 +1,82 @@ | |||
import { JSX } from 'react'; |
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.
why do we have react in the project?
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.
removed
Feature
added transaction tracking
Contains breaking changes
[x] No
[] Yes
Updated CHANGELOG
[x] Yes
Testing
[x] User testing
[] Unit tests