-
Notifications
You must be signed in to change notification settings - Fork 52
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
Fix guild names UI #950
Fix guild names UI #950
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
PR Reviewer Guide 🔍
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request introduces changes that improve code readability and simplify logic. However, careful attention should be given to the new decoding method in useEntities.tsx
and the handling of potentially undefined values in useGuilds.tsx
to ensure robustness.
Butler is in closed beta. Reply with feedback or to ask Butler to review other parts of the PR. Please give feedback with emoji reacts.
@@ -37,7 +37,7 @@ export const useEntities = () => { | |||
|
|||
const getEntityName = (entityId: bigint) => { | |||
const entityName = getComponentValue(EntityName, getEntityIdFromKeys([entityId])); | |||
return entityName ? hexToAscii(numberToHex(Number(entityName.name))) : entityId.toString(); | |||
return entityName ? shortString.decodeShortString(entityName.name.toString()) : entityId.toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that shortString.decodeShortString
correctly handles all expected input formats and edge cases. This change in decoding logic could potentially introduce bugs if not all scenarios are covered.
@@ -80,7 +80,7 @@ export const useGuilds = () => { | |||
(id) => getComponentValue(GuildMember, id), | |||
)[0]?.guild_entity_id; | |||
|
|||
const guildName = userGuildEntityId ? getEntityName(BigInt(userGuildEntityId!)) : undefined; | |||
const guildName = userGuildEntityId ? getEntityName(BigInt(userGuildEntityId)) : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that userGuildEntityId
is always defined where used, or handle potential undefined
cases. The removal of the non-null assertion simplifies the code but could lead to undefined behavior if not managed properly.
PR Code Suggestions ✨
|
User description
Closes #947
PR Type
Bug fix, Enhancement
Description
useEntities.tsx
to useshortString.decodeShortString
instead ofhexToAscii
.getEntityName
function call inuseGuilds.tsx
by removing unnecessary non-null assertion.formatGuilds
function inuseGuilds.tsx
.Changes walkthrough 📝
useEntities.tsx
Update entity name decoding method to use `shortString`.
client/src/hooks/helpers/useEntities.tsx
hexToAscii
import.shortString
import fromstarknet
.hexToAscii
withshortString.decodeShortString
for decodingentity names.
useGuilds.tsx
Simplify guild name retrieval and improve readability.
client/src/hooks/helpers/useGuilds.tsx
getEntityName
function call by removing unnecessarynon-null assertion.
formatGuilds
function.