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

Fix network alert (only for gchain for now) #505

Merged
merged 5 commits into from
May 4, 2022

Conversation

alfetopito
Copy link
Collaborator

Summary

Fixing network alert for gchain

From

Screen Shot 2022-05-03 at 13 37 48
Screen Shot 2022-05-03 at 13 37 42

To

Screen Shot 2022-05-03 at 13 26 19
Screen Shot 2022-05-03 at 13 26 13

To Test

  1. Connect to gchain
  2. Change between light/dark modes

@alfetopito alfetopito self-assigned this May 3, 2022
@alfetopito alfetopito requested review from a team May 3, 2022 12:39
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2022

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2022

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@elena-zh
Copy link

elena-zh commented May 3, 2022

Hey @alfetopito , nice changes!
However, I have found the issue here: the banner remains to be displayed when I switch from GC to an unsupported network
hide

I think, it would be great to hide this message when connected to an unsupported network.

Then, I think it would be nice to add a 'hover' effect to the banner (highlight links or so). WDYT?
image

@alfetopito
Copy link
Collaborator Author

@elena-zh should be fixed, please try again once the build finishes

@elena-zh
Copy link

elena-zh commented May 4, 2022

Hey @alfetopito , great!

However, I have noticed that only in this PR I have started to see 'Hang in here..' message when switching back from an unsupported network to a supported one.
hang

Please, follow these steps to reproduce the issue:

  1. Connect a wallet to GC
  2. Change to an aunsupported network
  3. Change the networj to Ethereum --> you will see a hanging 'Hang in here' message

As for the hover effect, could we please change its color to orange as we do on the 'Profile' page for cards?
orange

Thanks!

@alfetopito
Copy link
Collaborator Author

Hmmm, that's weird.
Locally everything is fine.
Trying it out in the PR link though I can reproduce it.
Will see what I can do.

@alfetopito
Copy link
Collaborator Author

In any way, that issue is definitely not related to the PR at hand and should be addressed separately.
I'll address your other comment regarding the orange in the link then proceed

@alfetopito alfetopito changed the title Dropping gradients for gchain network alert component Fix network alert (only for gchain for now) May 4, 2022
@elena-zh
Copy link

elena-zh commented May 4, 2022

@alfetopito , hm. I retested it in #502 PR, but it was not reproducible there. So I thought that it was related to the current PR.
But now I have retested it again in #502 pr, and now I'm able to reproduce it there. So I have created #514 issue for this.

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

LGTM now!

@alfetopito
Copy link
Collaborator Author

Merging as is, @fairlighteth feel free to post merge review

@alfetopito alfetopito merged commit d83a7d1 into release/1.14 May 4, 2022
@alfetopito alfetopito deleted the fix-gchain-network-warning branch May 4, 2022 13:50
@github-actions github-actions bot locked and limited conversation to collaborators May 4, 2022
@fairlighteth
Copy link
Contributor

Looks mostly good to me. One thing I noticed is a slight overlap of the Appzi button:
Screen Shot 2022-05-04 at 15 21 30

But it's something I can address at some point.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants