fix: disabled JoyID connector

[ahonn]

See #67 and #69

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
spore-demo	✅ Ready (Inspect)	Visit Preview	Feb 8, 2024 4:16am

[ShookLyngs]

Hi, I have a question: why not remove or comment out the new JoyIdConnector() from the connectors?

spore-demo/src/pages/_app.tsx

Lines 95 to 98 in 0d95ee8

	const config = {
	autoConnect: true,
	connectors: [new MetaMaskConnector(), new JoyIdConnector()],
	};

[ahonn]

Hi, I have a question: why not remove it from the JoyIdConnector from the connectors?

spore-demo/src/pages/_app.tsx

Lines 95 to 98 in 0d95ee8

	const config = {
	autoConnect: true,
	connectors: [new MetaMaskConnector(), new JoyIdConnector()],
	};

For users who have previously used JoyID connect, removing the JoyId connector directly may result in errors and unexpected inability to control Spore that was previously Minted.
Therefore, I am more inclined to only hide it for new users without affecting existing users who use JoyID for connect.

[ShookLyngs]

For users who have previously used JoyID connect, removing the JoyId connector directly may result in errors and unexpected inability to control Spore that was previously Minted. Therefore, I am more inclined to only hide it for new users without affecting existing users who use JoyID for connect.

Okay, now I understand. This is a simple change to remove the JoyID option from the wallet connect modal without affecting other users who have already connected via JoyID. However, if the connected user accidentally clicks the "disconnect" button, does it mean that the user will lose the ability to reconnect via JoyID thereafter?

For that, I recommend marking the connector as somewhat "deprecated" and still showing it in the wallet connect modal, but lowering its ordering. Additionally, add some text to explain why this option is currently not recommended. Do you think it would be a better approch for everyone?

[ahonn]

For users who have previously used JoyID connect, removing the JoyId connector directly may result in errors and unexpected inability to control Spore that was previously Minted. Therefore, I am more inclined to only hide it for new users without affecting existing users who use JoyID for connect.

Okay, now I understand. This is a simple change to remove the JoyID option from the wallet connect modal without affecting other users who have already connected via JoyID. However, if the connected user accidentally clicks the "disconnect" button, does it mean that the user will lose the ability to reconnect via JoyID thereafter?

For that, I recommend marking the connector as somewhat "deprecated" and still showing it in the wallet connect modal, but lowering its ordering. Additionally, add some text to explain why this option is currently not recommended. Do you think it would be a better approch for everyone?

There are currently only two connector available, so simply marking it as deprecated may not be sufficient.
My idea is to discourage future users from using the JoyID connector. The current changes has minimal impact.

I believe it is acceptable that existing users who disconnect from JoyID connect will no longer be able to use it, as the purpose of this change is to prevent any confusion for them. In the README, I have also clearly stated that we have temporarily hidden the integration of JoyID.

Perhaps we can consider show tips when existing users disconnect from JoyID connect, but I believe the necessity of doing so is relatively low. What do you think?

[ShookLyngs]

Perhaps we can consider show tips when existing users disconnect from JoyID connect, but I believe the necessity of doing so is relatively low. What do you think?

From my perspective, I believe it would be more reasonable to add a card/box in the ConnectModal to explain why the JoyID option is no longer visible (like you did in the README). This approach ensures that users understand the change and discourages them from attempting to connect using JoyID again, not when they're trying to disconnect.

[ahonn]

Perhaps we can consider show tips when existing users disconnect from JoyID connect, but I believe the necessity of doing so is relatively low. What do you think?

From my perspective, I believe it would be more reasonable to add a card/box in the ConnectModal to explain why the JoyID option is no longer visible. This approach ensures that users understand the change and discourages them from attempting to connect using JoyID again, not when they're trying to disconnect.

Cool, I think adding a message in ConnectModal to provide an explanation is better.

[ahonn]

@ShookLyngs Now we can still see JoyID in ConnectModal, but it is marked as disabled. We will display a message about JoyID being deprecated and link to #69

[ShookLyngs]

@ShookLyngs Now we can still see JoyID in ConnectModal, but it is marked as disabled. We will display a message about JoyID being deprecated and link to #69

Cool, I have a question: do you think it's a good idea to change the ordering of the elements in the modal? I feel like the current ordering (Message -> MetaMask -> JoyID) might hinder MetaMask-only users as they only want to connect via MetaMask.

Would it be better if we change the order to the following:

1. MetaMask
2. JoyID
3. Message

[ahonn]

@ShookLyngs Now we can still see JoyID in ConnectModal, but it is marked as disabled. We will display a message about JoyID being deprecated and link to #69

Cool, I have a question: do you think it's a good idea to change the ordering of the elements in the modal? I feel like the current ordering (Message -> MetaMask -> JoyID) might hinder MetaMask-only users as they only want to connect via MetaMask.

Would it be better if we change the order to the following:

1. MetaMask
2. JoyID
3. Message

Usually, announcements or notifications should be placed at the top, so I have followed this convention.
I don’t have a particular preference for where to place it, I think both options are fine.

[ShookLyngs]

Usually, announcements or notifications should be placed at the top, so I have followed this convention. I don’t have a particular preference for where to place it, I think both options are fine.

Yeah, I think if you wish the message to be eye-catching, the current ordering is just corret.
I have no other questions for it, thanks.