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

adding kyc link and connect button #792

Merged
merged 4 commits into from
Jul 22, 2021
Merged

adding kyc link and connect button #792

merged 4 commits into from
Jul 22, 2021

Conversation

josojo
Copy link
Contributor

@josojo josojo commented Jul 22, 2021

This adds the kyc link that can be maintained for each auctioning token

the KYC json list should be fetched from this github repo, instead of the build, but for reviewing the PR I am pulling it via internal reference.

If the account is not yet connected, currently the interface shows that the user is not allowlisted, which is confusing. I changed that behaviour as well and show the user the connect button.

@henrypalacios any proposal for improvements are highly welcome

@github-actions
Copy link

  • 🔭 IDO UI: Easy IDO auction

@elena-zh
Copy link

Hi @josojo , great job!
However, an empty place order section with 'Connect Wallet' button look s bit odd to me..
May be we should add some kind of a message 'Please connect your wallet to start' or so there?
connect a wallet

Another minor case: could we navigate a user to the https://qualify.swarm.markets/ in another tab?

Thanks!

Copy link
Contributor

@henrypalacios henrypalacios left a comment

Choose a reason for hiding this comment

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

@josojo I can't think of another scalable solution from the front end to add KYC file (maybe later we could do it from the auction creation).

The same things that @elena-zh already mentioned came to my mind.

  1. Leave comments below
  2. at least from swarm go back to the auction page maybe open a new tab

return (
<>
<Wrapper>
{auctionInfoLoading && <InlineLoading size={SpinnerSize.small} />}
{!auctionInfoLoading && isPrivate && !signatureAvailable && (
{!auctionInfoLoading && isPrivate && !signatureAvailable && account == null && (
Copy link
Contributor

Choose a reason for hiding this comment

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

it may also be confusing just to show the button Connect Wallet, how about changing the account logic to show that part?

        <PrivateWrapper>
          <LockBig />
          <TextBig>Private auction</TextBig>
          <EmptyContentTextNoMargin>
            You need to get allowed to participate.
          </EmptyContentTextNoMargin>
        </PrivateWrapper>
        {account == null ? (
          <ActionButton onClick={toggleWalletModal}>Connect Wallet</ActionButton>
        ) : (
          <EmptyContentTextSmall>
            {linkForKYC ? (
              <ExternalLink href={linkForKYC}>Get Allowed ↗</ExternalLink>
            ) : (
              <EmptyContentTextSmall>
                Ask the auctioneer to get allow-listed.
              </EmptyContentTextSmall>
            )}{' '}
          </EmptyContentTextSmall>
        )}

Selection_320

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I like this one. Leet's do it

<EmptyContentTextSmall>
{linkForKYC ? (
<ExternalLink href={linkForKYC}>
<h2>Get Allowed ↗</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the designers don't like to have a h2 here 😂, we could get the h2 style at LinkCSS or maybe use the same style of Connect Wallet button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should modify it. Though my css styles are very bad.
@maria-vslvn could you find a proper solution that looks somehow similar to the h2?

@henrypalacios
Copy link
Contributor

henrypalacios commented Jul 22, 2021

@elena-zh I have applied some changes, what do you think?

I have tried to keep the same pattern for the other cases

Selection_322

@maria-vslvn I'm not sure if the size is right here (Get Allowed text), and if you guys use relative units.

@josojo josojo merged commit 85e455c into develop Jul 22, 2021
@josojo josojo deleted the kycLink branch July 22, 2021 19:24
@elena-zh
Copy link

Great job!
I have created 2 low priority enhancements related to this pr: #794, #795

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

Successfully merging this pull request may close these issues.

3 participants