Skip to content

Conversation

@blacktoast
Copy link
Contributor

@blacktoast blacktoast commented Nov 12, 2025

Pull Request

Thank you for raising a Pull Request. Please follow the instruction.

  • I’ve read CONTRIBUTING.md and followed the guidelines.

Summary

구현 사항

  • To reference the latest state of isSignedIn in the callback, add a ref variable.
  • run yarn check to format

Links (Issue References, etc, if there's any)

okoCosmos
.getKey("cosmoshub-4")
.then((key) => setCosmosAddress(key.bech32Address)),
okoCosmos.getKey("cosmoshub-4").then((key) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

hardcoded string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think make it constant vars?
I just thought it is used just one time and in local

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it'd be better if we have const COSMOSHUB_4

const okoEth = useSDKState((state) => state.oko_eth);
const isSignedIn = useUserInfoState((state) => state.isSignedIn);
const isSignedRef = useRef(isSignedIn);
isSignedRef.current = isSignedIn;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To reference the latest state of isSignedIn in the callback, add a ref variable.

^ as I mentioned PR, Within the callback function, it is necessary to retrieve the most recent isSignedIn state.

Copy link
Collaborator

@eldenpark eldenpark Nov 17, 2025

Choose a reason for hiding this comment

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

@blacktoast I still don't see why this ref is necessary. Could we hop on a short call when you are available?

@eldenpark
Copy link
Collaborator

btw, this has some conflicts. Could you take a look into it? @blacktoast

@blacktoast blacktoast requested a review from eldenpark November 14, 2025 11:38
@@ -0,0 +1 @@
export const COSMOS_CHAIN_ID = "cosmoshub-4";
Copy link
Collaborator

Choose a reason for hiding this comment

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

constants can be anything. Too general this file is.

@eldenpark eldenpark merged commit 7d1788b into main Nov 17, 2025
2 checks passed
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