feat: switch chain dropdown #527
Conversation
WalkthroughNetworkStatus was moved from the AccountStatus trigger into the header tray and replaced with a Dropdown-wrapped interactive badge that opens a SwitchChainMenu; the menu attempts async chain switches via requestSwitchChain with error reporting and updated badge/menu styling, preserving Skeleton rendering when no chain is present. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NetworkStatusUI as NetworkStatus (UI)
participant AppLogic as requestSwitchChain / useDrizzle
participant Wallet as Wallet Provider
User->>NetworkStatusUI: Click badge → open Dropdown
User->>NetworkStatusUI: Select target chain
NetworkStatusUI->>AppLogic: requestSwitchChain(targetChain)
AppLogic->>Wallet: RPC: wallet_switchEthereumChain / request
Wallet-->>AppLogic: Success / Error
AppLogic-->>NetworkStatusUI: Resolve / reject result
NetworkStatusUI->>User: Show success or message.error on failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Deploy Preview for kleros-court ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/network-status.js (1)
74-86: Consider extracting shared button styling.Lines 77-86 duplicate the styling from
StyledAccountStatusinaccount-status.js(background-color, border-radius, padding, hover states). Both components now share identical interactive button styling.Consider creating a shared styled component or CSS mixin:
// In a shared styles file or at the top of a component file const InteractiveButtonStyles = css` cursor: pointer; background-color: rgba(255, 255, 255, 0.15); border-radius: 24px; padding: 8px 12px; font-weight: 400; transition: all 0.2s ease-in; :hover, :focus { background-color: rgba(255, 255, 255, 0.25); } `; // Then use it in both components: const StyledBadge = styled(Badge)` white-space: nowrap; ${InteractiveButtonStyles} // ... rest of specific styles `;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/bootstrap/app.js(2 hunks)src/components/account-status.js(1 hunks)src/components/network-status.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/account-status.js (1)
src/components/eth-address.js (1)
ETHAddress(8-21)
src/bootstrap/app.js (1)
src/components/network-status.js (1)
NetworkStatus(41-58)
src/components/network-status.js (3)
src/bootstrap/app.js (1)
StyledMenu(300-327)src/api/side-chain/request-switch-chain.js (1)
requestSwitchChain(6-18)src/helpers/networks.js (2)
chainIdToNetworkShortName(8-13)chainIdToNetworkShortName(8-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - kleros-court
- GitHub Check: Header rules - kleros-court
- GitHub Check: Pages changed - kleros-court
🔇 Additional comments (6)
src/components/account-status.js (1)
21-23: LGTM! Clean refactoring.The simplification correctly removes NetworkStatus from the account trigger, as it's now rendered separately in the header tray.
src/bootstrap/app.js (2)
11-11: LGTM!The NetworkStatus import is correctly added to support rendering the network indicator in the header tray.
126-126: LGTM! Good placement in the header tray.The NetworkStatus component is now rendered alongside AccountStatus and NotificationSettings, providing a consistent location for network information.
src/components/network-status.js (3)
47-54: LGTM! Good implementation of the dropdown trigger.The badge is properly wrapped in a Dropdown component, making the network status clickable and showing available chains on click.
70-72: LGTM!Simple, clean styling for the dropdown menu.
9-9: The import is correct. Thesrc/api/side-chain/index.jsre-exportsrequestSwitchChainas a named export (line 7:export { default as requestSwitchChain } from "./request-switch-chain"), so the named import syntax{ requestSwitchChain }is valid and will work correctly at runtime.Likely an incorrect or invalid review comment.
tractorss
left a comment
tractorss
left a comment
There was a problem hiding this comment.
Network change does not handle Addition of Chiado chain
It's because of this check which checks if it's supported chain, and chiado is not added to the list of supported lists
kyrers
force-pushed
the
feat/swith-chain-drodpown
branch
from
68be488 to
878a430
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/account-status.js (1)
20-25: Avoid an empty clickable pill in view‑only mode.When
account === VIEW_ONLY_ADDRESS, the trigger renders an emptyStyledAccountStatusbutton, which is clickable but has no visible or accessible label. Consider showing a placeholder (e.g., “View-only” / “Not connected”) or disabling the button in that state:- {account !== VIEW_ONLY_ADDRESS ? ( - <span className="address"> - <ETHAddress address={account} withLink={false} /> - </span> - ) : null} + {account !== VIEW_ONLY_ADDRESS ? ( + <span className="address"> + <ETHAddress address={account} withLink={false} /> + </span> + ) : ( + <span className="address">View-only</span> + )}Adjust the placeholder copy as fits your UX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/bootstrap/app.js(2 hunks)src/components/account-status.js(1 hunks)src/components/network-status.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/bootstrap/app.js (1)
src/components/network-status.js (1)
NetworkStatus(42-59)
src/components/account-status.js (2)
src/components/eth-address.js (1)
ETHAddress(8-21)src/components/required-chain-id-modal.js (1)
account(14-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - kleros-court
- GitHub Check: Header rules - kleros-court
- GitHub Check: Pages changed - kleros-court
🔇 Additional comments (3)
src/bootstrap/app.js (1)
11-11: NetworkStatus integration in the header tray looks correct.Importing
NetworkStatusand placing it inStyledTraybeforeAccountStatusensures it’s rendered within bothDrizzleProviderandDrizzleChainIdProvider, so it has the contexts it needs and matches the “switch chain dropdown in the header” requirement.Also applies to: 126-129
src/components/network-status.js (2)
5-11: SwitchChainMenu logic and error handling look solid.Filtering out the current chain, short‑circuiting when
targetChainId === chainId, and surfacing failures viamessage.errorgives a clean UX and avoids redundant RPC calls. The use ofchainIdToNetworkShortNamekeeps the menu options in sync with the displayed badge text.Also applies to: 14-40
71-83: Dropdown and badge styling match the header pill pattern.The rounded
StyledMenuand pill‑styleStyledBadge(background, padding, border radius, hover/focus transition) align visually withAccountStatus, keeping the header tray cohesive while making the network badge look clearly interactive.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/network-status.js (1)
55-61: Make the Dropdown trigger keyboard‑accessible with a real button.
StyledBadgeultimately renders a non-focusable element (e.g.,span), so keyboard users can’t tab to or activate the network switcher even though it’s clickable with a mouse.Wrap the badge in a button trigger and add an accessible label:
return chainId ? ( <Dropdown overlay={<SwitchChainMenu />} trigger={["click"]}> - <StyledBadge - className={clsx(status, className)} - status={networkStatusToBadgeStatus[status]} - text={chainIdToNetworkShortName[chainId]} - /> + <StyledTrigger + type="button" + aria-label={`Switch network (current: ${chainIdToNetworkShortName[chainId] || "unknown"})`} + > + <StyledBadge + className={clsx(status, className)} + status={networkStatusToBadgeStatus[status]} + text={chainIdToNetworkShortName[chainId]} + /> + </StyledTrigger> </Dropdown> ) : (And define the trigger styles near the other styled components:
const StyledMenu = styled(Menu)` border-radius: 4px; `; +const StyledTrigger = styled.button` + padding: 0; + border: none; + background: transparent; + cursor: pointer; +`; + const StyledBadge = styled(Badge)`After this change, please verify in-browser that:
- The trigger is reachable via Tab.
- Enter/Space opens the dropdown.
- A visible focus indicator is present when the trigger is focused.
🧹 Nitpick comments (2)
src/components/network-status.js (2)
14-46: NormalizechainIdtype to avoid redundant self-switches and keep menu options clean.If
useChainId()ever returns a string (e.g.,"1"), the strict comparisons in the filter and inhandleSwitchChainwill fail, and the current network will still appear as an option and trigger an unnecessary switch request. Normalizing to numbers in this component makes the logic robust regardless of the hook’s return type.You can harden this block like so:
function SwitchChainMenu() { const { drizzle } = useDrizzle(); - const chainId = useChainId(); + const chainId = Number(useChainId()); const handleSwitchChain = async (targetChainId) => { if (targetChainId === chainId) return; // Already on this chain @@ - {Object.entries(switchableChains) - .filter(([key]) => Number(key) !== chainId) - .map(([key, value]) => ( - <Menu.Item key={key} onClick={() => handleSwitchChain(Number(key))}> - <span>{value}</span> - </Menu.Item> - ))} + {Object.entries(switchableChains) + .filter(([key]) => Number(key) !== chainId) + .map(([key, value]) => { + const targetId = Number(key); + return ( + <Menu.Item key={targetId} onClick={() => handleSwitchChain(targetId)}> + <span>{value}</span> + </Menu.Item> + ); + })}Optionally, to avoid duplicating labels, you could derive
switchableChainsfromchainIdToNetworkShortName:const switchableChains = { };Please verify how
useChainIdis currently typed/implemented and adjust if needed.
83-93: Hover/focus styling is good; consider aligning it with the actual focusable element.The hover/focus background changes on
StyledBadgelook nice, but once the trigger is abutton, the keyboard focus will be onStyledTrigger, not the badge. If you want keyboard focus to mirror the hover visuals, you might move (or duplicate) the:hover, :focusstyles ontoStyledTriggerinstead of the badge.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/network-status.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/network-status.js (3)
src/bootstrap/app.js (1)
StyledMenu(300-327)src/api/side-chain/request-switch-chain.js (1)
requestSwitchChain(6-18)src/helpers/networks.js (2)
chainIdToNetworkShortName(8-13)chainIdToNetworkShortName(8-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - kleros-court
- GitHub Check: Header rules - kleros-court
- GitHub Check: Pages changed - kleros-court
🔇 Additional comments (2)
src/components/network-status.js (2)
5-11: Imports and hook wiring look consistent.All newly added imports (
Badge,Dropdown,Menu,message,Skeleton,requestSwitchChain,useDrizzle) are used and keep the file’s responsibilities cohesive; no issues here.
77-80: Menu styling change is safe and improves visual polish.Adding a border radius on
StyledMenuis unobtrusive and consistent with the rest of the header tray styling; no functional concerns here.
Resolves #526.
PR-Codex overview
This PR introduces a new
NetworkStatuscomponent and modifies theAccountStatusandNetworkStatuscomponents to enhance network management features, including chain switching functionality.Detailed summary
NetworkStatuscomponent insrc/bootstrap/app.js.NetworkStatusfromAccountStatus.NetworkStatus.NetworkStatuswith dropdown for switching chains.StyledBadgeand addedStyledMenu.Summary by CodeRabbit
New Features
UI/UX Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.