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

Disabled the trade panel if Optimism is not connected #1416

Merged
merged 5 commits into from
Sep 26, 2022
Merged

Conversation

LeifuChen
Copy link
Contributor

@LeifuChen LeifuChen commented Sep 22, 2022

Description

Disable the trade panel if the wallet is not connected or the network is unsupported

Related issue

Kwenta/kwenta-private#324

Motivation and Context

Make sure the user can only interact with the trade panel when connecting the optimism mainnet or optimism goerli.

How Has This Been Tested?

Case 1: No wallet connected

  1. Visit the futures page
  2. The trade panel is replaced by the message connect wallet
  3. Click on the connect wallet
  4. The trade panel is visible after connecting the wallet

Case 2: Connected to Polygon

  1. Visit the futures page
  2. The trade panel is replaced by the message Switch to L2
  3. Click on the Switch to L2
  4. The trade panel is visible after switching to L2

Screenshots (if appropriate):

image
image

@vercel
Copy link

vercel bot commented Sep 22, 2022

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

Name Status Preview Updated
kwenta ✅ Ready (Inspect) Visit Preview Sep 23, 2022 at 2:21PM (UTC)

@LeifuChen LeifuChen marked this pull request as draft September 22, 2022 22:06
@vercel vercel bot temporarily deployed to Preview September 22, 2022 22:06 Inactive
@LeifuChen LeifuChen marked this pull request as ready for review September 22, 2022 22:33
@LeifuChen LeifuChen changed the title Disabled the trade panel if L2 is not connected Disabled the trade panel if Optimism is not connected Sep 22, 2022
@vercel vercel bot temporarily deployed to Preview September 22, 2022 22:34 Inactive
@platschi platschi self-requested a review September 22, 2022 22:41
@platschi
Copy link
Contributor

platschi commented Sep 23, 2022

Works fine so far, just one small nitpick (Switch network button colors should be responsive to theme:)

Screen Shot 2022-09-23 at 09 02 28

Nice catch. Will fix this.

Copy link
Contributor

@avclarke avclarke left a comment

Choose a reason for hiding this comment

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

Looks good, the only other thing I was thinking is we can also replace the CrossMarginUnsupported with this so we have a single component to handle this across both futures types.

@vercel vercel bot temporarily deployed to Preview September 23, 2022 14:17 Inactive
@vercel vercel bot temporarily deployed to Preview September 23, 2022 14:21 Inactive
@LeifuChen
Copy link
Contributor Author

Looks good, the only other thing I was thinking is we can also replace the CrossMarginUnsupported with this so we have a single component to handle this across both futures types.

Sounds like a good idea. @platschi do you want to have a more generic component in the new PR or in this PR?

@avclarke
Copy link
Contributor

avclarke commented Sep 23, 2022

Looks good, the only other thing I was thinking is we can also replace the CrossMarginUnsupported with this so we have a single component to handle this across both futures types.

Sounds like a good idea. @platschi do you want to have a more generic component in the new PR or in this PR?

I don't mind updating the cross margin part actually in a separate PR as I have a couple of ideas to improve the state there in general.

@platschi platschi merged commit 8bb8ece into dev Sep 26, 2022
@platschi platschi deleted the fix/switch-network branch September 26, 2022 12:34
@KngZhi KngZhi mentioned this pull request Oct 5, 2022
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