sygmaprotocol / sygma-widget

Transfer widget for the sygmaprotocol
5 stars 2 forks source link

feat: enable developers to configure network whitelisting #170

Closed Lykhoyda closed 6 months ago

Lykhoyda commented 7 months ago

Closes #148 #146

Lykhoyda commented 7 months ago

Is there a reason for putting this into context? We are passing it only to the first child (<sygma-fungible-transfer), while context is used to avoid passing it trough multiple levels

@mpetrunic I think the main reason, for now, is consistency as we have other config params provided by the user in "sygma-config-context-provider", so to keep the centralized place for all the user params. I also can imagine that we could need this information in other components in the future, so probably makes sense to have it all together, although indeed it will be easier to pass the props.

Lykhoyda commented 7 months ago

To test this story make changes in index.html file by adding or removing the networks or resources:

    <script>
      const widget = document.getElementById('sygmaprotocolWidget');
      widget.whitelistedSourceNetworks = ['sepolia', 'cronos'];
      widget.whitelistedDestinationNetworks = ['mumbai', 'sepolia', 'rococo-phala'];
      widget.whitelistedSourceResources = ['ERC20LRTest'];
    </script>
mpetrunic commented 7 months ago

Is there a reason for putting this into context? We are passing it only to the first child (<sygma-fungible-transfer), while context is used to avoid passing it trough multiple levels

@mpetrunic I think the main reason, for now, is consistency as we have other config params provided by the user in "sygma-config-context-provider", so to keep the centralized place for all the user params. I also can imagine that we could need this information in other components in the future, so probably makes sense to have it all together, although indeed it will be easier to pass the props.

I would keep it out of the context until there is need for it. Makes testing easier and reduces chance of additional rerenders (context is event based)

LyonSsS commented 7 months ago

Tested on PR https://github.com/sygmaprotocol/sygma-widget/pull/169 with a mix of valid and invalid or missing form sharedconfig.json data Valid image Valid - since Mumbai was removed and Sepolia can't send to Sepolia image image image Valid image image

Lykhoyda commented 7 months ago

In react if you update, for example, whitelistedSourceNetworks. That update won't be reflected in the component. Fungible token transfer component should listen for field updates and reinit transfer controller on change

Got it, removed the attribute: false 👍

mpetrunic commented 6 months ago

In react if you update, for example, whitelistedSourceNetworks. That update won't be reflected in the component. Fungible token transfer component should listen for field updates and reinit transfer controller on change

Got it, removed the attribute: false 👍

@Lykhoyda Please reread my comment haha

Fungible token transfer component should listen for field updates and reinit transfer controller on change