-
Notifications
You must be signed in to change notification settings - Fork 82
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
frontend: utilize send-wrapper to lift some stuff from the Send
component
#2955
base: master
Are you sure you want to change the base?
frontend: utilize send-wrapper to lift some stuff from the Send
component
#2955
Conversation
Send
component
@@ -31,7 +31,7 @@ export const ConfirmingWaitDialog = ({ | |||
selectedUTXOs, | |||
coinCode, | |||
transactionDetails | |||
}: Omit<TConfirmSendProps, 'device'>) => { | |||
}: Omit<TConfirmSendProps, 'bb01Paired'>) => { |
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.
when you add bb01Paired to TConfirmSendProps you need to Omit it 2 times.
How about you remove it from TConfirmSendProps and add it only where needed, something like:
diff --git a/frontends/web/src/routes/account/send/components/confirm/confirm.tsx b/frontends/web/src/routes/account/send/components/confirm/confirm.tsx
index 5c1d5b05e..7b4d19451 100644
--- a/frontends/web/src/routes/account/send/components/confirm/confirm.tsx
+++ b/frontends/web/src/routes/account/send/components/confirm/confirm.tsx
@@ -30,8 +30,13 @@ type TFiatValueProps = {
amount: string
}
-export const ConfirmSend = (props: TConfirmSendProps) => {
- return (props.bb01Paired !== undefined ? (
+type ConfirmSend = { bb01Paired: boolean | undefined } & TConfirmSendProps;
+
+export const ConfirmSend = ({
+ bb01Paired,
+ ...props
+}: ConfirmSend) => {
+ return (bb01Paired !== undefined ? (
<ConfirmingWaitDialog
{...props}
/>
@@ -50,7 +55,7 @@ export const BB02ConfirmSend = ({
selectedUTXOs,
coinCode,
transactionDetails
-}: Omit<TConfirmSendProps, 'bb01Paired'>) => {
+}: TConfirmSendProps) => {
const { t } = useTranslation();
const {
diff --git a/frontends/web/src/routes/account/send/components/confirm/dialogs/confirm-wait-dialog.tsx b/frontends/web/src/routes/account/send/components/confirm/dialogs/confirm-wait-dialog.tsx
index 6235aea0e..9b8a27cec 100644
--- a/frontends/web/src/routes/account/send/components/confirm/dialogs/confirm-wait-dialog.tsx
+++ b/frontends/web/src/routes/account/send/components/confirm/dialogs/confirm-wait-dialog.tsx
@@ -31,7 +31,7 @@ export const ConfirmingWaitDialog = ({
selectedUTXOs,
coinCode,
transactionDetails
-}: Omit<TConfirmSendProps, 'bb01Paired'>) => {
+}: TConfirmSendProps) => {
const { t } = useTranslation();
const [signProgress, setSignProgress] = useState<TSignProgress>();
diff --git a/frontends/web/src/routes/account/send/components/confirm/types.ts b/frontends/web/src/routes/account/send/components/confirm/types.ts
index 6e288db04..dd280e389 100644
--- a/frontends/web/src/routes/account/send/components/confirm/types.ts
+++ b/frontends/web/src/routes/account/send/components/confirm/types.ts
@@ -27,7 +27,6 @@ export type TransactionDetails = {
}
export type TConfirmSendProps = {
- bb01Paired: boolean | undefined;
baseCurrencyUnit: ConversionUnit;
note: string;
hasSelectedUTXOs: boolean;
@@ -516,20 +501,16 @@ class Send extends Component<Props, State> { | |||
</Column> | |||
</Grid> | |||
</ViewContent> | |||
|
|||
{device && ( |
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.
With this ConfirmSend is always mounted, I think it is fine...
but I want to test this a bit.
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 could also change the meaning of bb01Paired: boolean | undefined
to undefined
== no device connected and then use true/false
to check whether its a bb01 or bb02
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.
nvm, that does not work
Utilize the `SendWrapper` to get the mobile channel error, and (bb01) paired status in case a bb01 is connected. This simplifies the `Send` component and better distinguishes code for bb01/bb02 stuff.
ca49784
to
e544037
Compare
Type `coinUnit` in `IAccount` to use `CoinUnit`.
e544037
to
a03270f
Compare
@@ -25,12 +25,14 @@ import { TConfirmSendProps } from '@/routes/account/send/components/confirm/type | |||
import { ConfirmingWaitDialog } from '@/routes/account/send/components/confirm/dialogs/confirm-wait-dialog'; | |||
import style from './confirm.module.css'; | |||
|
|||
type TConfimrSend = { bb01Paired: boolean | undefined } & TConfirmSendProps; |
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 landed in the wrong commit? Seems not related to "type coinUnit in IAccount "
Two small improvements towards the functional refactor.
Two things:
bitbox-wallet-app/frontends/web/src/routes/account/send/send.tsx
Lines 134 to 137 in 9ed7888
Into the send-wrapper and I'm not so sure if that's a good idea. But imo. there is no point in showing the error/returning only when the user hits send, because that is the ultimate goal of the component anyways.
bb01Paired
instead ofdevice
(which was actuallyproduct
) now to decide whether to show BB01 or BB02 stuff. This is a bit implicit but I think it is easy to understand/does not make the code harder to understand. Ofc. we can just add the device prop. back in if you think it is worth it.