feat: externalize wallet connect connection through props

sygmaprotocol / sygma-widget

Transfer widget for the sygmaprotocol

5 stars 2 forks source link

feat: externalize wallet connect connection through props #94

Closed wainola closed 5 months ago

wainola commented 5 months ago

Description

Related Issue Or Context

Closes: #86

How Has This Been Tested? Testing details.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Documentation

Checklist:

[ ] I have commented my code, particularly in hard-to-understand areas.
[ ] I have ensured that all acceptance criteria (or expected behavior) from issue are met
[ ] I have updated the documentation locally and in sygma-docs.
[ ] I have added tests to cover my changes.
[ ] I have ensured that all the checks are passing and green, I've signed the CLA bot

wainola commented 5 months ago

@mpetrunic can you give it a first look and provide feedback pls? 😄

wainola commented 5 months ago

It probably makes sense to define context for global widget options instead of passing them to each component

This is a great idea

wainola commented 5 months ago

It probably makes sense to define context for global widget options instead of passing them to each component

@mpetrunic seeking clarification here: do you mean context for all the widget options that we currently defined as properties or just the ones concerning the wallet connection? Because if this is true for the former, wouldn't make sense to tackle that on a different PR with it's own issue?

mpetrunic commented 5 months ago

It probably makes sense to define context for global widget options instead of passing them to each component

@mpetrunic seeking clarification here: do you mean context for all the widget options that we currently defined as properties or just the ones concerning the wallet connection? Because if this is true for the former, wouldn't make sense to tackle that on a different PR with it's own issue?

I guess, for now just theme options and wallet options^^

wainola commented 5 months ago

btw @mpetrunic I think we might need to expose the ISygmaProtocolWidget from the react package, because with the react example, there is no type inferring when for instance defining a dynamic theme with setState

mpetrunic commented 5 months ago

btw @mpetrunic I think we might need to expose the ISygmaProtocolWidget from the react package, because with the react example, there is no type inferring when for instance defining a dynamic theme with setState

Thats fine