fix: don't shorten ens names #431
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces modifications across multiple components to standardize the display of recipient addresses by replacing the utility function Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
src/components/Dashboard/index.tsx (1)
115-115: Approved: Address display improvedThe change from
utils.shortenAddressLong(address)toutils.printableAddress(address)aligns with the PR objective of not shortening ENS names. This improvement ensures that ENS names are displayed in full, enhancing user experience and readability.Consider adding a comment explaining the behavior of
utils.printableAddressfor future maintainers, e.g.:// utils.printableAddress displays full ENS names or shortened addresses `See all links created and claimed ${address ? `with ${utils.printableAddress(address)}` : 'on this device'}`src/components/Claim/Link/Onchain/Confirm.view.tsx (2)
187-187: LGTM! Consider adding a comment for clarity.The change from
utils.shortenAddressLongtoutils.printableAddressaligns well with the PR objective of not shortening ENS names. This modification ensures that ENS names are displayed in full while still potentially shortening regular addresses.Consider adding a brief comment explaining the purpose of
utils.printableAddress, such as:// Use printableAddress to display full ENS names or shortened addresses {recipient.name ? recipient.name : utils.printableAddress(recipient.address ?? '')}This would help future developers understand the intent behind using this specific function.
Line range hint
1-324: Overall impact: Improved address display without side effectsThe change to use
utils.printableAddressinstead ofutils.shortenAddressLongsuccessfully addresses the PR objective of not shortening ENS names. This modification enhances the user experience by displaying ENS names in full while likely still shortening regular addresses.The localized nature of this change means that:
- It doesn't introduce any apparent negative impacts on the component's functionality or performance.
- No other modifications to the component were necessary to accommodate this change.
To further improve the component's robustness and maintainability, consider:
- Documenting the expected behavior of
utils.printableAddressin the project's documentation or as a comment near its implementation.- Adding unit tests to verify that ENS names and regular addresses are displayed correctly in various scenarios.
src/components/Create/Link/Confirm.view.tsx (1)
189-189: LGTM! Consider a minor refactor for clarity.The change successfully implements the requirement to not shorten ENS names while standardizing the display of addresses using
utils.printableAddress. This aligns well with the PR objectives.For improved readability, consider extracting the address display logic into a separate function:
const getDisplayAddress = (name: string | undefined, address: string | undefined) => { if (name?.endsWith('.eth')) return name; return utils.printableAddress(address ?? ''); }; // Then use it in the JSX: `Send to ${getDisplayAddress(recipient.name, recipient.address)}`This refactoring would make the code more maintainable and easier to understand at a glance.
src/components/Profile/Components/TableComponent.tsx (1)
Verify address handling across additional files
The initial change in
TableComponent.tsxis effective. However, address shortening is also used in multiple other locations. To ensure consistency across the codebase, please update the following instances to use the newprintableAddressfunction:
src/utils/general.utils.tssrc/components/Request/Pay/Views/Success.view.tsxsrc/components/Claim/Generic/SenderClaim.view.tsxsrc/components/Request/Pay/Views/Initial.view.tsxsrc/components/Claim/Link/Initial.view.tsxsrc/components/Claim/Link/Onchain/Confirm.view.tsxsrc/components/Claim/Link/Onchain/Success.view.tsxsrc/components/Global/Header/index.tsxsrc/components/Global/ImageGeneration/LinkPreview.tsxUpdating these instances will ensure uniform address handling throughout the application.
🔗 Analysis chain
Line range hint
1-268: Verify address handling across the codebaseThe change in this component looks good, but to fully meet the PR objective, we should ensure that all instances of address display throughout the codebase are using the new
printableAddressfunction.Run the following script to identify other potential locations where address shortening might still be in use:
If any results are found, consider updating those instances to use
printableAddressas well.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances of address shortening across the codebase rg --type typescript 'shortenAddress|shortenAddressLong'Length of output: 93
Script:
#!/bin/bash # Search for instances of address shortening across TypeScript files rg --glob "*.ts" --glob "*.tsx" 'shortenAddress|shortenAddressLong'Length of output: 2281
src/utils/general.utils.ts (1)
20-23: LGTM: NewprintableAddressfunction addresses ENS name handling.The new
printableAddressfunction effectively addresses the PR objective of not shortening ENS names. It provides a consistent way to display addresses, whether they are ENS names or regular addresses. The implementation is concise and clear.Consider adding a check for null or undefined input to make the function more robust:
export const printableAddress = (address: string): string => { + if (!address) return ''; if (address.endsWith('.eth')) return address return shortenAddressLong(address) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- src/components/Claim/Link/Onchain/Confirm.view.tsx (1 hunks)
- src/components/Create/Link/Confirm.view.tsx (1 hunks)
- src/components/Create/Link/Initial.view.tsx (1 hunks)
- src/components/Create/Link/Input.view.tsx (1 hunks)
- src/components/Create/Link/Success.view.tsx (1 hunks)
- src/components/Dashboard/components/MobileComponent.tsx (1 hunks)
- src/components/Dashboard/index.tsx (2 hunks)
- src/components/Profile/Components/TableComponent.tsx (1 hunks)
- src/components/Profile/index.tsx (4 hunks)
- src/components/Request/Pay/Views/Success.view.tsx (1 hunks)
- src/utils/general.utils.ts (2 hunks)
🧰 Additional context used
📓 Learnings (1)
src/components/Request/Pay/Views/Success.view.tsx (3)
Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#409 File: src/components/Request/Pay/Views/Success.view.tsx:23-23 Timestamp: 2024-10-08T20:13:44.480Z Learning: In the `SuccessView` component, `transactionHash` remains constant because we are in a success view, so it's unnecessary to include it in the dependency array of `useEffect` hooks.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#409 File: src/components/Request/Pay/Views/Success.view.tsx:23-23 Timestamp: 2024-10-08T20:13:42.967Z Learning: In the `SuccessView` component, `transactionHash` remains constant because we are in a success view, so it's unnecessary to include it in the dependency array of `useEffect` hooks.Learnt from: jjramirezn PR: peanutprotocol/peanut-ui#409 File: src/components/Request/Pay/Views/Success.view.tsx:23-23 Timestamp: 2024-10-03T12:52:52.740Z Learning: In the `SuccessView` component, `transactionHash` remains constant because we are in a success view, so it's unnecessary to include it in the dependency array of `useEffect` hooks.
🔇 Additional comments (14)
src/components/Request/Pay/Views/Success.view.tsx (2)
29-29: Approved: Improved address handlingThe change from a conditional check to
utils.printableAddress(requestLinkData.recipientAddress)is a good improvement. This modification aligns with the PR objective of not shortening ENS names and simplifies the code. TheprintableAddressfunction is likely designed to handle both regular addresses and ENS names appropriately, ensuring consistent and correct display across the application.
Line range hint
23-27: Correct: useEffect dependency arrayThe useEffect hook correctly omits
transactionHashfrom its dependency array. This aligns with the previous learnings, which state that in a success view,transactionHashremains constant. Therefore, it doesn't need to be included in the dependency array. Good job maintaining this implementation!src/components/Dashboard/components/MobileComponent.tsx (1)
47-47: LGTM! VerifyprintableAddressimplementation.The change from
utils.shortenAddressLongtoutils.printableAddressaligns well with the PR objective of not shortening ENS names. This modification ensures that both regular addresses and ENS names are handled appropriately in the UI.To ensure the correct implementation of
printableAddress, please run the following script:This will help confirm that the
printableAddressfunction is correctly implemented and consistently used across the codebase.✅ Verification successful
printableAddressimplementation verified.The
printableAddressfunction is correctly implemented insrc/utils/general.utils.tsand is consistently used across the codebase, ensuring that ENS names are not shortened as intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of printableAddress function # Test: Check the implementation of printableAddress ast-grep --lang typescript --pattern 'function printableAddress($_) { $$$ }' # Test: Verify usage of printableAddress across the codebase rg --type typescript 'printableAddress'Length of output: 155
Script:
#!/bin/bash # Description: Verify the implementation of printableAddress function # Test: Check the implementation of printableAddress ast-grep --lang typescript --pattern 'function printableAddress($_) { $$$ }' # Test: Verify usage of printableAddress across the codebase rg --glob "*.ts" --glob "*.tsx" 'printableAddress'Length of output: 2404
src/components/Create/Link/Initial.view.tsx (1)
234-234: LGTM! Verify ENS name handling.The change from
utils.shortenAddressLong(recipient.address, 6)toutils.printableAddress(recipient.address)aligns with the PR objective of not shortening ENS names. This implementation should correctly handle both regular addresses and ENS names.To ensure the correct handling of ENS names, please run the following verification script:
This script will help us confirm that the
printableAddressfunction is correctly implemented and consistently used across the codebase.✅ Verification successful
Verified: ENS names are correctly handled without being shortened. The
printableAddressfunction accurately distinguishes between ENS names and regular addresses, ensuring ENS names remain intact in the UI.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of printableAddress function # Test: Check the implementation of printableAddress rg --type typescript 'function printableAddress' -A 10 # Test: Verify usage of printableAddress across the codebase rg --type typescript 'printableAddress'Length of output: 166
Script:
#!/bin/bash # Description: Locate the implementation of printableAddress function using file extension # Test: Find the implementation of printableAddress in .ts and .tsx files rg 'function printableAddress' --glob '*.ts' --glob '*.tsx' -A 10 # Test: Find all usages of printableAddress in .ts and .tsx files rg 'printableAddress' --glob '*.ts' --glob '*.tsx' -A 5Length of output: 10264
src/components/Dashboard/index.tsx (2)
Line range hint
1-287: Summary: Address display improvements implemented successfullyThe changes in this file consistently replace
utils.shortenAddressLongwithutils.printableAddressfor displaying addresses. This modification aligns well with the PR objective of not shortening ENS names, potentially improving user experience and readability throughout the dashboard.Key points:
- The changes are minimal and focused, reducing the risk of introducing new bugs.
- The modifications are applied consistently in both the dashboard label and the table display.
- Existing fallback logic using nullish coalescing operators is preserved.
To ensure a comprehensive implementation:
- Verify that
utils.printableAddressis correctly implemented to handle both ENS names and regular addresses.- Check for any other occurrences of address shortening in the codebase that might need similar updates.
- Consider adding unit tests for the
utils.printableAddressfunction to ensure its correct behavior with various input types.
187-187: Approved: Consistent address display in tableThe change from
utils.shortenAddressLongtoutils.printableAddressis consistent with the previous modification and aligns with the PR objective of not shortening ENS names. This ensures a uniform display of addresses throughout the dashboard.To ensure the correct behavior of
utils.printableAddress, please run the following verification script:This script will help confirm that
utils.printableAddressis correctly implemented and consistently used across the codebase.✅ Verification successful
Further Verification Needed: Issues with Current Verification Scripts
The initial verification attempts did not yield results due to unrecognized file types and no matches found for
utils.printableAddress. To accurately verify the implementation and usage ofutils.printableAddress, please run the following updated verification scripts:These scripts aim to locate the
printableAddressfunction by removing file type restrictions and explicitly searching within.tsand.tsxfiles. This should help determine whether the function exists and is properly utilized across the codebase.
Further Verification Needed: Unable to Locate
utils.printableAddressFunction DefinitionThe initial verification scripts did not locate the
printableAddressfunction definition, likely due to differing function declaration styles or incorrect file type specifications. To accurately verify the implementation and usage ofutils.printableAddress, please run the following updated verification scripts:These scripts aim to identify the
printableAddressfunction regardless of its declaration style and ensure that the correct file types are targeted during the search. This will help confirm the existence and implementation details ofutils.printableAddressacross the codebase.
Verified:
utils.printableAddressis correctly implemented and consistently usedThe
utils.printableAddressfunction is properly defined insrc/utils/general.utils.tsand is consistently utilized across multiple components within the codebase. The change fromutils.shortenAddressLongtoutils.printableAddressensures a uniform display of addresses, aligning with the PR objective.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of utils.printableAddress # Test: Check if utils.printableAddress exists and handles both ENS names and regular addresses ast-grep --lang typescript --pattern $'function printableAddress(address: string): string { $$$ }' # Test: Verify usage of printableAddress in other components rg --type typescript 'utils\.printableAddress'Length of output: 185
Script:
#!/bin/bash # Description: Verify the implementation of utils.printableAddress with adjusted file types # Test: Check if utils.printableAddress exists and handles both ENS names and regular addresses ast-grep --lang typescript --pattern $'function printableAddress(address: string): string { $$$ }' # Test: Verify usage of printableAddress in other components without specifying file type rg 'utils\.printableAddress'Length of output: 2315
Script:
#!/bin/bash # Description: Alternative verification for utils.printableAddress # Test: Search for printableAddress function in .ts and .tsx files explicitly fd --extension ts --extension tsx 'printableAddress' | xargs grep -H 'function printableAddress' # Test: List all TypeScript files to ensure correct file type usage fd --extension ts --extension tsxLength of output: 9484
Script:
#!/bin/bash # Description: Enhanced verification of utils.printableAddress implementation # Test 1: Check for function declaration in various styles ast-grep --lang typescript --pattern $'function printableAddress(address: string): string { $$$ }' ast-grep --lang typescript --pattern $'const printableAddress = (address: string): string => { $$$ }' ast-grep --lang typescript --pattern $'export const printableAddress = (address: string): string => { $$$ }' # Test 2: Verify usage of printableAddress across .ts and .tsx files rg 'utils\.printableAddress' --type ts --type tsxLength of output: 944
Script:
#!/bin/bash # Description: Alternative verification for utils.printableAddress with corrected file types # Test 1: Search for printableAddress function declarations as constants or exports rg 'const printableAddress\s*=' --type ts --type tsx rg 'export\s+const printableAddress\s*=' --type ts --type tsx # Test 2: List all TypeScript and TSX files to ensure correct file type usage fd --extension ts --extension tsxLength of output: 9562
src/components/Profile/Components/TableComponent.tsx (1)
111-111: LGTM! Verify ENS name handling.The change from
utils.shortenAddressLongtoutils.printableAddressaligns with the PR objective of not shortening ENS names. This should correctly handle both regular addresses and ENS names.To ensure the change works as intended, please verify:
- The
printableAddressfunction in the utils file correctly identifies and preserves ENS names.- Test the component with both regular addresses and ENS names to confirm proper rendering.
src/components/Profile/index.tsx (5)
80-80: LGTM: Consistent address displayThe change from
utils.shortenAddressLongtoutils.printableAddressaligns with the PR objective of not shortening ENS names. This ensures a consistent and appropriate display of addresses throughout the component.
274-274: LGTM: Consistent contact address displayThe replacement of
utils.shortenAddressLongwithutils.printableAddressfor contact addresses is consistent with the previous change. This ensures that ENS names for contacts are displayed appropriately without being shortened.
395-395: LGTM: Consistent user identifier displayThe use of
utils.printableAddressinstead ofutils.shortenAddressLongfor the user's email or account identifier is consistent with the previous changes. This ensures that if an ENS name is used as the account identifier, it will be displayed appropriately without being shortened.
564-564: LGTM: Consistent referral address display and overall improvementThe use of
utils.printableAddressfor referral addresses is consistent with the previous changes, ensuring that ENS names for referrals are displayed appropriately without being shortened.Overall, these changes successfully implement the PR objective of not shortening ENS names throughout the Profile component. The consistent use of
utils.printableAddressacross different parts of the component (initial user name, contacts, user identifier, and referrals) ensures a uniform and correct display of addresses and ENS names.
Line range hint
80-564: Summary: Successful implementation of ENS name handlingThis PR successfully implements the objective of not shortening ENS names throughout the Profile component. The consistent replacement of
utils.shortenAddressLongwithutils.printableAddressacross various parts of the component ensures that:
- User account identifiers are displayed correctly
- Contact addresses are shown appropriately
- User's own email or account identifier is handled properly
- Referral addresses are presented without shortening
These changes improve the user experience by maintaining the integrity of ENS names in the UI. The implementation is consistent and well-executed across the component.
src/utils/general.utils.ts (2)
1-5: LGTM: Import statements updated appropriately.The removal of the
interfacesimport and the addition of thepeanutimport from the SDK are appropriate changes. This suggests a shift towards using the SDK directly, which can lead to better consistency and easier maintenance.
12-18: LGTM: Improved type safety forshortenAddressLong.The addition of an explicit return type
: stringto theshortenAddressLongfunction enhances type safety and improves code readability. The function logic remains unchanged, maintaining existing behavior.
|
|
Check everywhere when the addresses are shortened if we have an ens. If we have a ens we should not shorten it