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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

NicolaLS
Copy link
Contributor

@NicolaLS NicolaLS commented Oct 3, 2024

Two small improvements towards the functional refactor.

Two things:

  1. I moved this:
    if (this.state.noMobileChannelError) {
    alertUser(this.props.t('warning.sendPairing'));
    return;
    }

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.

  1. I use bb01Paired instead of device (which was actually product) 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.

@NicolaLS NicolaLS changed the title Send improvements before functional refactor frontend: utilize send-wrapper to lift some stuff from the Send component Oct 4, 2024
@@ -31,7 +31,7 @@ export const ConfirmingWaitDialog = ({
selectedUTXOs,
coinCode,
transactionDetails
}: Omit<TConfirmSendProps, 'device'>) => {
}: Omit<TConfirmSendProps, 'bb01Paired'>) => {
Copy link
Collaborator

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;

@NicolaLS NicolaLS force-pushed the send-improvements-before-functional-refactor branch 2 times, most recently from e544037 to a03270f Compare October 14, 2024 14:24
@NicolaLS NicolaLS force-pushed the send-improvements-before-functional-refactor branch 4 times, most recently from 0b71ef7 to 7eb4b77 Compare October 28, 2024 19:25
@NicolaLS
Copy link
Contributor Author

@thisconnect PTAL

if (account && product && product === 'bitbox') {
const fetchData = async () => {
try {
const mobileChannel = await hasMobileChannel(product)();
Copy link
Collaborator

@thisconnect thisconnect Nov 25, 2024

Choose a reason for hiding this comment

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

with this PR I get a 404 :(

Screenshot 2024-11-25 at 16 40 13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, it is wrong to pass product to hasMobileChannel not sure why I did that. I'll update it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same for getDeviceInfo actually

@NicolaLS NicolaLS force-pushed the send-improvements-before-functional-refactor branch from 7eb4b77 to c5af9f1 Compare November 27, 2024 01:22
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.
Type `coinUnit` in `IAccount` to use `CoinUnit`.
@NicolaLS NicolaLS force-pushed the send-improvements-before-functional-refactor branch from c5af9f1 to 1f60566 Compare November 27, 2024 01:23
@NicolaLS
Copy link
Contributor Author

@thisconnect thanks for the review, it was just a stupid logical error hope it works now. forgot to rebase/push before the change so the last push is the rebase and the one before the edit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants