-
-
Notifications
You must be signed in to change notification settings - Fork 257
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
fix(suite): fix coinjoin pausing with several ongoing sessions #6934
Conversation
import { COINJOIN_PHASE_MESSAGES } from '@suite-constants/coinjoin'; | ||
import { | ||
calculateSessionProgress, | ||
getPhaseTimerFormat, | ||
getSessionDeadlineFormat, | ||
} from '@wallet-utils/coinjoinUtils'; | ||
import { useDispatch } from 'react-redux'; |
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 know it's not a first usage, but would be nice to discuss what is the current preferred way https://www.notion.so/satoshilabs/Redux-b0a4c3b7e71b499a8d592fb44cd794fe#31445739321c401dbf1d2396ee4ad3bc
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.
Far from the first one, I've been using it for several months, I think 😅 Also, the mobile team has been using it for a while. But we can definitely discuss it and compare pros and cons.
packages/suite/src/components/wallet/CoinjoinSummary/CoinjoinStatus.tsx
Outdated
Show resolved
Hide resolved
@@ -8,4 +8,4 @@ | |||
|
|||
Resolve <!--- link the issue here --> | |||
|
|||
## Screenshots (if appropriate): | |||
## Screenshots: |
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.
This looks like totally unrelated to this PR.
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.
It is, I think this change is not really worth CI time...
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.
Totally agree, that unrelated things should be in separate PRs. No need to change it for this PR, but for the future it's better not to save the CI time that much.
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.
LGTM, please rebase on develop and resolve conflicts
76c7b1d
to
f2e7210
Compare
@dahaca please check the failing unit test |
f2e7210
to
7d597c6
Compare
Description
Related Issue
Resolve #6812
Screenshots:
Screen.Recording.2022-11-16.at.15.42.02.mov