Skip to content

[feat] add all option in contacts #1399

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

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Conversation

blacktoast
Copy link
Collaborator

구현사항

  • RecipientInput, RecipientInputForStarknet에 onChangeCallback를 추가해서 선택적으로 이전에 필요한 작업을 처리 할 수있게 수정
  • enableNameserviceWhenPrefixEntered옵션을 recipientConfig 추가해서 chain id가 all일때 ens, icns에서 체인 구분자 까지 입력되야 nameService가 동작 되도록 수정

Copy link

vercel bot commented Mar 19, 2025

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

Name Status Preview Comments Updated (UTC)
keplr-wallet-extension ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 19, 2025 4:59am

@Thunnini Thunnini changed the base branch from master to develop March 19, 2025 13:04
Copy link
Member

@Thunnini Thunnini left a comment

Choose a reason for hiding this comment

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

흠 이거 구현이 현재 상태의 recipient config로는 좀 어려워보여서 내일 좀 얘기해보시죠

@@ -264,6 +278,31 @@ export class RecipientConfig
setValue(value: string): void {
this._value = value;

if (this._option?.enableNameserviceWhenPrefixEntered) {
Copy link
Member

Choose a reason for hiding this comment

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

솔직히 실행은 안시켜봤지만 enableNameserviceWhenPrefixEntered 구현에서 이 부분에서 처리했을때 문제가 예상되는게 recipientConfig의 value와 name service들의 value가 sync가 맞아야 하는데 이렇게 구현했을때 예를들어 dogemos.cosmos를 유저가 입력했다가 마지막 글자를 지워서 dogemos.cosmo로 바꿨을때 name service랑 sync가 안맞게 되지 않을까 싶습니다.
확실하게 하려면 좀 귀찮지만 enableNameserviceWhenPrefixEntered 옵션을 각각의 name service implementation에 다 넣어서 처리해야할 듯합니다. 그리고 setValue에서 처리하는게 아니라 nameSerivce.result getter에서 처리하거나 해야할듯한데 이 부분은 좀 더 봐야할듯 합니다

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.

2 participants