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: use postmessage to communicate with the iframe #3146

Conversation

thisconnect
Copy link
Collaborator

The previous approach depended on the iframe having access to the parent context, but this wont work when the iframe is hosted on a different domain.

Instead of relaxing webengine restrictions this change simply uses postMessage api to pass data from and to the iframe.

@thisconnect thisconnect force-pushed the frontend-btcdirect-postmessage branch 2 times, most recently from 76b35fd to adbd456 Compare January 29, 2025 14:04
@thisconnect thisconnect requested a review from bznein January 30, 2025 07:30
textContent: message,
})
);
}

})();
</script>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the file we host on shiftcrypto.io and is already deployed bitboxapp.shiftcrypto.io/widgets/btcdirect/v1/fiat-to-coin.html

but in order for it to communicate with the react app we need the changes in this PR in btcdirect.tsx

@thisconnect
Copy link
Collaborator Author

thisconnect commented Jan 30, 2025

temporary cherry-picked commit from AddressFromBackend branch

bznein and others added 2 commits January 30, 2025 11:04
Provide an endpoint to be called from the frontend.
This endpoints returns the APIKey, the URL address
and, if the action is "buy", the address.
The previous approach depended on the iframe having access to the
parent context, but this wont work when the iframe is hosted on a
different domain.

Instead of relaxing webengine restrictions this change simply uses
postMessage api to pass data from and to the iframe.
@thisconnect thisconnect force-pushed the frontend-btcdirect-postmessage branch from 5375e85 to 1f645e8 Compare January 30, 2025 10:04
@thisconnect thisconnect removed the request for review from bznein January 30, 2025 10:07
@thisconnect thisconnect marked this pull request as draft January 30, 2025 10:07
@thisconnect
Copy link
Collaborator Author

@Beerosagos @bznein I changed to draft and cherry-picked the backend change from #3142 to test this in the qt app. The widget loads 🎉

let me know if you want to test both changes or if I should drop the commit again so this is ready for review (but can't be tested as it needs the backend change)

token: apiKey,
debug: mode === 'debug',
locale: window.frameElement?.dataset.locale || 'en-GB',
theme: window.frameElement?.dataset.theme || 'light',
Copy link
Collaborator Author

@thisconnect thisconnect Feb 3, 2025

Choose a reason for hiding this comment

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

TODO: take locale and theme from onMessage "configuration" / event.data

Copy link
Collaborator Author

@thisconnect thisconnect Feb 3, 2025

Choose a reason for hiding this comment

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

rebased this branch without the backend commit and fixed "locale" and "theme" in #3152


window.addEventListener('btcdirect-embeddable-fiat-to-coin-order-confirmed', (event) => {
console.log('btcdirect-embeddable-fiat-to-coin-order-confirmed', event);
// Handle the event
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you could directly post a message to pass the order confirmation, maybe? And leave the TODO in the handler inside btcdirect.tsx. No strong opinion, tho

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I only tested once, this event is fired after the user clicked proceed to payment and not really useful this way, I'll just remove.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed in #3152

const locale = i18n.resolvedLanguage ? localeMapping[i18n.resolvedLanguage] : 'en-GB';

const onMessage = useCallback((event: MessageEvent) => {
// if (!isDevserver && event.origin !== 'https://shiftcrypto.io') { return; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this commented?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll update once we have the backend part, the correct subdomain is https://bitboxapp.shiftcrypto.io.

If you don't mind I'll continue in #3152

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated in #3152

theme: isDarkMode ? 'dark' : 'light',
baseCurrency: getCoinCode(account.coinCode),
quoteCurrency: 'EUR',
mode: isTesting || debug ? 'debug' : 'production',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this correct? I didn't test it (no pun intended 😇 ) but I think that isTesting is true for testnet, but we could use mainnet on the sandbox too, right? Maybe this is the cause of the apikey error you mentioned here? #3142 (comment)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as discussed in DM it would be great to align with the backend, so we need something like isProdservers in the frontend.

Alternative: maybethe backend could pass 'debug' or 'production' in another field in some existing api call.

@thisconnect
Copy link
Collaborator Author

closing in favor of #3152

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.

3 participants