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

feat: change input of name TNS register #978

Merged
merged 5 commits into from
Feb 22, 2024
Merged

Conversation

VishalRakholiya-iView
Copy link
Collaborator

No description provided.

Copy link

netlify bot commented Feb 19, 2024

Deploy Preview for testitori ready!

Name Link
🔨 Latest commit 83b483c
🔍 Latest deploy log https://app.netlify.com/sites/testitori/deploys/65d73f397abc6c00085b1cc9
😎 Deploy Preview https://deploy-preview-978--testitori.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 19, 2024

Deploy Preview for teritori-dapp ready!

Name Link
🔨 Latest commit 83b483c
🔍 Latest deploy log https://app.netlify.com/sites/teritori-dapp/deploys/65d73f396a96eb00089f08f5
😎 Deploy Preview https://deploy-preview-978--teritori-dapp.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't create a specific component, you just copy what they done in CreateDAOSection.tsx but it's better to have an input component that check available names, that call the TextInputCustom etc. and called this component in CreateDAOSection and TNSNameFinderModal

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you mean use TextInputCustom in both file (CreateDAOSection and TNSNameFinderModal)?

and write logic in TextInputCustom ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, create a specific input components in packages/components/inputs that called TextInputCustom and write logic in it. And after that call this specific component in CreateDAOSection and TNSNameFinderModal

Copy link
Collaborator

@n0izn0iz n0izn0iz left a comment

Choose a reason for hiding this comment

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

I agree with clemsss, please extract the component

Comment on lines 113 to 124
const selectedWallet = useSelectedWallet();
const [network] = parseUserId(selectedWallet?.userId);
const networkId = network?.id;
const cosmosNetwork = getCosmosNetwork(networkId);
const normalizedTokenId = (
name + cosmosNetwork?.nameServiceTLD || ""
).toLowerCase();

const tokenId = name + cosmosNetwork?.nameServiceTLD || "";

const { nsMintPrice: price } = useNSMintPrice(networkId, normalizedTokenId);
const { nameAvailable, loading } = useNSMintAvailability(networkId, tokenId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

all this should be inside the AvailableNamesInput component

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep you have to move the logic too in the component, if it's possible, but i think it is

Copy link
Collaborator

@clegirar clegirar left a comment

Choose a reason for hiding this comment

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

When you moved the logic in the new component, please clean comments in the code

Comment on lines 113 to 124
const selectedWallet = useSelectedWallet();
const [network] = parseUserId(selectedWallet?.userId);
const networkId = network?.id;
const cosmosNetwork = getCosmosNetwork(networkId);
const normalizedTokenId = (
name + cosmosNetwork?.nameServiceTLD || ""
).toLowerCase();

const tokenId = name + cosmosNetwork?.nameServiceTLD || "";

const { nsMintPrice: price } = useNSMintPrice(networkId, normalizedTokenId);
const { nameAvailable, loading } = useNSMintAvailability(networkId, tokenId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep you have to move the logic too in the component, if it's possible, but i think it is

import {
neutral33,
neutral77,
primaryColor,
redDefault,
// primaryColor,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just remove this comments

@clegirar
Copy link
Collaborator

And please pass the CI, don't export TextInputCustomProps

@clegirar
Copy link
Collaborator

The only thing i noticed in the UI is the equivalent in $
Screenshot 2024-02-22 132129
This one is from create-org screen
Screenshot 2024-02-22 132139
This one is from tns

Please add the equivalent in dollars in the custom component you created 👍

Copy link
Collaborator

@clegirar clegirar left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@n0izn0iz n0izn0iz merged commit 1bcafbb into main Feb 22, 2024
19 checks passed
@n0izn0iz n0izn0iz deleted the tns-choosing-name-UI branch February 22, 2024 15:22
@VishalRakholiya-iView VishalRakholiya-iView restored the tns-choosing-name-UI branch February 23, 2024 09:29
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