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

Will/deeplink-bridge #208

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

Will/deeplink-bridge #208

wants to merge 2 commits into from

Conversation

camerow
Copy link
Contributor

@camerow camerow commented Dec 11, 2024

No description provided.

Copy link

Deploying lockstacks with  Cloudflare Pages  Cloudflare Pages

Latest commit: edc0a8e
Status: ✅  Deploy successful!
Preview URL: https://5839d3cf.lockstacks.pages.dev
Branch Preview URL: https://will-deeplink-bridge.lockstacks.pages.dev

View logs

Copy link
Collaborator

@kyranjamie kyranjamie left a comment

Choose a reason for hiding this comment

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

LGTM bar one comment


const leatherDetected = window.LeatherProvider !== undefined;

export const useLeatherSBTCBridgeButton = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

define as fn, and consider sbtc a word to avoid SBTCB capitalisation

Suggested change
export const useLeatherSBTCBridgeButton = () => {
export function useLeatherSbtcBridgeButton() {

if (!leatherDetected) {
return;
}
(window as any).LeatherProvider?.request('swap', { from: 'STX', to: 'ALEX' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kyranjamie FYI the only reason I had to do this any business is because the @leather.io/rpc types don't include this method - is that on purpose? I have also been updating the package as I add these things and plan to push those changes and move our types here over to that.

Copy link
Collaborator

@kyranjamie kyranjamie Dec 12, 2024

Choose a reason for hiding this comment

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

@kyranjamie FYI the only reason I had to do this any business is because the @leather.io/rpc types don't include this method - is that on purpose?

Have we included the override of window types anywhere? .request should be there then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.request is in there but the parameters have types and open is not defined in our request parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's what I mean: leather-io/mono#728

@fabric-8
Copy link
Contributor

fabric-8 commented Dec 12, 2024

Case: Leather wallet not installed
Can we use this modal?

image

Design doesn't have to be on par, we can use the stacks connect/install one and just apply the copy/content (with leather being the only wallet avail to install)

@@ -31,8 +31,7 @@ export function ChooseStackingMethodLayout(props: ChooseStackingMethodLayoutProp
<Messages {...props} />
</Box>
)}

{/* <EarnWithSBTCSection {...props} /> */}
<EarnWithSBTCSection {...props} />
Copy link

Choose a reason for hiding this comment

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

Suggested change
<EarnWithSBTCSection {...props} />
<EarnWithSbtcSection {...props} />

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can only unhide this by monday/official sBTC launch, otherwise this'll already get visible to everybody once merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the domain is currently basically private (not linked anywhere) so i think we're safe to merge soonish - we will have to update marketing links to this page as a part of this release, as well as redirect lockstacks.com -> earn

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.

4 participants