Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Add link read more in footer, and inform about changing networks #2265

Merged
merged 4 commits into from
Jan 24, 2022

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Jan 23, 2022

Summary

Before

image

After

Add the read more link below the button:
image

also here:
image

Additionally, hint the user they might have a claiming available in another Network
image

To Test

  1. Make sure the new links work

@anxolin anxolin requested review from a team January 23, 2022 18:50
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

@fairlighteth
Copy link
Contributor

My first impression is, that the text link we use right below the primary button, is seen as a 'secondary' action. The 'Read more about vCOW' feels like it should be part of the description above instead.

@elena-zh
Copy link

Links work for me!
However, I'd add a network name in the description as we do when search for a token in token lists:
image
image

@elena-zh elena-zh self-requested a review January 24, 2022 10:18
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.

I noticed duplicating 'Read more' links on screens where Airdrop option is available to claim:
image
It would be great to remove one of them.

@@ -43,7 +43,7 @@ export default function CanUserClaimMessage({ hasClaims, isAirdropOnly, handleCh
<ButtonSecondary onClick={handleChangeAccount} padding="0">
Try another account
</ButtonSecondary>{' '}
or <ExternalLink href={COW_LINKS.vCowPost}>read more about vCOW</ExternalLink>
or try in a different network.
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with Elena, show what's the current network being checked

@anxolin anxolin mentioned this pull request Jan 24, 2022
@anxolin
Copy link
Contributor Author

anxolin commented Jan 24, 2022

Added the network name
image

Also remove the duplicated link

@anxolin
Copy link
Contributor Author

anxolin commented Jan 24, 2022

Please review in the consolidated version :)

...merging

@anxolin anxolin merged commit f4dd347 into develop Jan 24, 2022
@alfetopito alfetopito deleted the read-more-link branch January 25, 2022 00:12
@elena-zh
Copy link

LGTM!

@W3stside
Copy link
Contributor

Approved

@fairlighteth
Copy link
Contributor

Looks good. Two comments:

  • We could later add a background color to the network name, using the same color as in the network switcher.
  • Is it ...in {network} or ...on {network} ? For example: "Claims on Gnosis Chain".

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.

5 participants