-
Notifications
You must be signed in to change notification settings - Fork 20
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: EVM contract details according to acceptance test #1237
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Skipped Deployments
|
WalkthroughThis update introduces several UI and configuration enhancements. The changelog now records a bug fix for the EVM contract details layout on mobile. The secondary color palette in the theme has been updated. The TypeSwitch component now scrolls to the top on tab changes, and the error message block has been removed from the verification process. Multiple EVM contract details components are updated to improve their layout and verify status display. Interaction components now feature responsive design adjustments, updated button styles, and mobile-specific behavior with revised proxy verification handling. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant TS as TypeSwitch Component
participant DOC as Document ("content" element)
U->>TS: Click on a new tab
TS->>TS: Trigger onTabChange callback
TS->>DOC: Scroll content element to the top
sequenceDiagram
participant U as User
participant ECD as EvmContractDetails Page
participant UM as useMobile Hook
participant ED as EvmContractDetailsTop
U->>ECD: Visit contract details page
ECD->>UM: Check if device is mobile
UM-->>ECD: Return isMobile flag
ECD->>ED: Pass updated props (including isVerified) and render tab label ("Read" for mobile)
U->>ED: Click "Read/Write" button
ED->>Router: Navigate to selected route based on contract address
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/lib/components/TypeSwitch.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/lib/pages/evm-contract-details/components/EvmContractDetailsTop.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.json". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib/components/TypeSwitch.tsx (1)
24-27
: Consider using React refs instead of direct DOM manipulation.While the current implementation works, using
document.getElementById
for DOM manipulation in React is not ideal. Consider using React refs for a more idiomatic approach.Here's an improved implementation using refs:
-const content = document.getElementById("content"); -if (content) { - content.scrollTo({ top: 0 }); -} +const contentRef = useRef<HTMLDivElement>(null); +contentRef.current?.scrollTo({ top: 0 });Remember to:
- Add the ref import:
import { useRef } from "react";
- Pass the ref to the content element:
<div ref={contentRef} id="content">
CHANGELOG.md (1)
89-90
: Changelog Entry Consistency CheckThe newly added entry for [#1237] "Fix EVM contract details UI layout and mobile" is clearly formatted and placed under the Bug fixes section. Please ensure that this entry follows the same style and punctuation conventions as the other entries in this section (e.g., the use of a hyphen at the start and proper linking of the PR number). If any additional context or details from the acceptance tests would enhance understanding for future readers, consider including a brief extra note in the description or in accompanying documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CHANGELOG.md
(1 hunks)src/config/theme/initia.ts
(1 hunks)src/lib/components/TypeSwitch.tsx
(1 hunks)src/lib/components/modal/evm-verify-status/EvmVerifyProcess.tsx
(0 hunks)src/lib/pages/evm-contract-details/components/EvmContractDetailsTop.tsx
(2 hunks)src/lib/pages/evm-contract-details/components/evm-contract-details-contract-info/ContractCode.tsx
(3 hunks)src/lib/pages/evm-contract-details/components/evm-contract-details-overview/OverviewVerifiedCmds.tsx
(3 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx
(4 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/index.tsx
(1 hunks)src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx
(5 hunks)src/lib/pages/evm-contract-details/index.tsx
(5 hunks)
💤 Files with no reviewable changes (1)
- src/lib/components/modal/evm-verify-status/EvmVerifyProcess.tsx
✅ Files skipped from review due to trivial changes (3)
- src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/index.tsx
- src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-write/WriteBox.tsx
- src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/index.tsx
🔇 Additional comments (15)
src/lib/pages/evm-contract-details/components/evm-contract-details-overview/OverviewVerifiedCmds.tsx (1)
31-74
: LGTM! Layout improvements align with PR objectives.The spacing and layout adjustments improve the mobile experience while maintaining a clean interface.
src/lib/pages/evm-contract-details/components/EvmContractDetailsTop.tsx (2)
44-58
: LGTM! Clean implementation of verification status display.The addition of the verification icon provides clear visual feedback about the contract's verification status.
89-91
: Verify the intentional hiding of Write button on mobile.The Write button is hidden on mobile devices (
display={{ base: "none", md: "block" }}
). Please confirm if this is intentional and aligns with the mobile UX requirements.src/config/theme/initia.ts (1)
31-36
: LGTM! Color palette update improves visual harmony.The new secondary color palette provides better contrast and a more cohesive visual hierarchy, which should enhance readability and user experience.
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx (3)
100-103
: LGTM! Mobile-specific behavior enhancement.Setting default interaction type to "read" on mobile devices improves usability by simplifying the interface for smaller screens.
117-134
: LGTM! Enhanced proxy contract UI.The proxy contract interface has been improved with:
- Clear implementation address display
- Better mobile responsiveness with truncated addresses
- Comprehensive verification info integration
28-28
:❓ Verification inconclusive
Verify proxy contract verification info handling.
The change from
proxyTargetAbi
toproxyTargetEvmVerifyInfo
suggests a more comprehensive verification approach. Please ensure all proxy contract interactions properly handle the new verification info structure.Also applies to: 36-36, 45-46
🏁 Script executed:
#!/bin/bash # Search for any remaining uses of proxyTargetAbi rg "proxyTargetAbi" -A 3Length of output: 1664
Action Required: Verify Consistent Handling of Proxy Verification Info
The update in
src/lib/pages/evm-contract-details/components/interact-evm-contract/index.tsx
correctly switches from usingproxyTargetAbi
toproxyTargetEvmVerifyInfo
. However, in related components—particularly inOverviewVerifiedCmds.tsx
—please ensure that all proxy contract interactions consistently derive their ABI and related verification details fromproxyTargetEvmVerifyInfo
. Although the call tocategorizeAbi(proxyTargetEvmVerifyInfo?.abi ?? [])
is correctly in place, the derived variables (proxyTargetAbiRead
andproxyTargetAbiWrite
) still carry the legacy naming. Please verify that:
- All logic based on the old
proxyTargetAbi
is now reliably sourced fromproxyTargetEvmVerifyInfo
.- The naming (and any associated documentation) in
OverviewVerifiedCmds.tsx
is updated or, if retained for backward compatibility, clearly justified.- Edge cases (such as when
proxyTargetEvmVerifyInfo
might be undefined) are appropriately handled across both read and write operations.This review is to ensure comprehensive consistency in proxy contract verification handling throughout the affected files.
src/lib/pages/evm-contract-details/index.tsx (2)
153-153
: LGTM! Improved mobile tab label.Simplifying the tab label to "Read" on mobile devices improves space utilization while maintaining clarity.
128-131
: LGTM! Enhanced verification status handling.Explicitly passing isVerified status to EvmContractDetailsTop improves the component's ability to render appropriate UI elements based on verification state.
src/lib/pages/evm-contract-details/components/interact-evm-contract/abi-read/ReadBox.tsx (3)
120-126
: LGTM! Responsive grid layout enhancement.The updated grid template provides better organization of content across different screen sizes.
137-165
: LGTM! Improved button layout for mobile.The flex container's responsive properties ensure better button alignment and spacing on mobile devices.
197-215
: LGTM! Enhanced copy button accessibility.The reorganized copy button layout improves accessibility and usability, especially on mobile devices.
src/lib/pages/evm-contract-details/components/evm-contract-details-contract-info/ContractCode.tsx (3)
3-3
: LGTM!The new imports are correctly added and used in the constructor arguments display section.
Also applies to: 8-8
35-38
: LGTM! Good refactoring.Moving the constructor arguments extraction outside the JSX improves code readability by separating the logic from the rendering.
88-96
: LGTM! Clean UI implementation.The conditional rendering provides a clear and user-friendly display of constructor arguments with appropriate styling and fallback message.
Summary by CodeRabbit
Bug Fixes
New Features
Style
Documentation