Conversation
📝 WalkthroughWalkthroughThis update introduces a comprehensive sessions management feature to the application, including the ability to open, view, and close network sessions via the UI. A new "Sessions" page and modal dialog are added, with Redux store enhancements to support session data, session actions, and minimum ticket probability retrieval. The navigation drawer and related UI components are updated to include session information and loading indicators. Several modals and tables are extended to integrate the session-opening functionality. The Redux logic is expanded with new async thunks and actions for session and probability management. Additional improvements include code formatting, type enhancements, dependency updates, and minor UI adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Redux
participant SDK
User->>UI: Click "Open Session"
UI->>UI: Show OpenSessionModal
User->>UI: Fill form, submit
UI->>Redux: Dispatch openSessionThunk
Redux->>SDK: Call openSession API
SDK-->>Redux: Return result
Redux-->>UI: Update sessions state
UI->>User: Show success/error, close modal
UI->>Redux: Dispatch getSessionsThunk
Redux->>SDK: Fetch sessions
SDK-->>Redux: Return sessions data
Redux-->>UI: Update sessions table
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
🧹 Nitpick comments (14)
src/future-hopr-lib-components/PeerInfo/index.tsx (1)
31-35: Enhanced Destructuring in PeersInfoThe multi-line destructuring of
props(extractingpeerId,nodeAddress, and the rest via...rest) increases readability. Note that whilepeerIdandnodeAddressare immediately extracted, other props (e.g.shortenPeerIdandshortenPeerIdIfAliasPresent) are still accessed viaprops. For uniformity, consider destructuring all used properties if that suits the project style.src/future-hopr-lib-components/Modal/index.tsx (1)
39-48: Refined Component Props Destructuring in ModalThe
Modalcomponent now destructures its props (onClose,selectedValue,open,title,children,maxWidth, anddisableScrollLock) in a multi-line format, enhancing readability. As a side note, the use of@ts-ignorein thehandleClosefunction suggests there may be a type mismatch; consider revisiting the type definitions foronCloseto eliminate the need for suppression.src/future-hopr-lib-components/Modal/styled.tsx (1)
4-7: Enhanced Destructuring Improves ReadabilityThe multi-line destructuring of the props (extracting
maxWidthand the rest) in theSDialogcomponent improves code clarity and maintainability without altering functionality.src/components/Modal/node/FundChannelModal.tsx (1)
113-118: Minor typesafety improvement opportunityWhile the keyboard event handler works correctly, consider using the more specific
KeyboardEventtype instead ofanyfor better type safety.-function handleEnter(event: any) { +function handleEnter(event: KeyboardEvent) {src/pages/node/configuration.tsx (3)
65-70: Consider updating the useEffect dependency array.The dependency array only includes
localNotificationSettings, but thehandleEnterfunction also depends oncanSave. This could potentially lead to stale closures where the event handler uses outdated values.- }, [localNotificationSettings]); + }, [localNotificationSettings, canSave]);
134-138: Consider using a more specific type for the event parameter.The
event: anytype is too generic and loses type safety. Using the importedKeyboardEventtype would be more appropriate.-function handleEnter(event: any) { +function handleEnter(event: KeyboardEvent) { - if (canSave && (event as KeyboardEvent).key === 'Enter') { + if (canSave && event.key === 'Enter') { handleSaveSettings(); } }
168-168: Remove console.log statement.This console.log statement should be removed before merging to production.
- console.log('localNotificationSettings', localNotificationSettings);src/store/slices/node/actionsFetching.ts (1)
89-94: Good implementation of reducer cases for new actions.The reducer cases for the new actions are well-implemented and follow the same pattern as existing cases. However, there's a minor formatting issue with the semicolon placement.
builder.addCase(setRedeemAllTicketsFetching, (state, action) => { state.redeemAllTickets.isFetching = action.payload; - }); + }), builder.addCase(setResetTicketStatisticsFetching, (state, action) => { state.resetTicketStatistics.isFetching = action.payload; });src/components/Modal/node/SendMessageModal.tsx (3)
116-122: Good addition of thecanSendMessagevariable but check the logic.The variable neatly encapsulates multiple conditions for enabling the send button, improving code readability. However, there may be a logical issue in one of the conditions:
(sendMode !== 'directMessage' && sendMode !== 'automaticPath' && numberOfHops < 0 && path === '')Should this be using
||between thenumberOfHops < 0andpath === ''parts? As written, both conditions must be true, but the intent might be that either condition should disable sending.Verify the intent of this condition, especially when
sendModeis neither 'directMessage' nor 'automaticPath'.
132-137: Consider updating the useEffect dependency array.The dependency array doesn't include
canSendMessagewhich is used in thehandleEnterfunction. This could potentially lead to stale closures.- }, [loginData, message, selectedReceiver, sendMode]); + }, [loginData, message, selectedReceiver, sendMode, canSendMessage]);
282-287: Consider improving thehandleEnterfunction.There's unnecessary type casting and a console.log statement that should be removed before merging to production.
function handleEnter(event: KeyboardEvent) { - if (canSendMessage && (event as KeyboardEvent)?.key === 'Enter') { + if (canSendMessage && event.key === 'Enter') { - console.log('SendMessageModal event'); handleSendMessage(); } }src/components/Modal/node/WithdrawModal.tsx (2)
116-121: Consider updating the useEffect dependency array.The dependency array includes all necessary state variables, but doesn't include
canWithdrawwhich is used in thehandleEnterfunction. This could potentially lead to stale closures.- }, [recipient, amount, apiEndpoint, apiToken, currency, loginData]); + }, [recipient, amount, apiEndpoint, apiToken, currency, loginData, canWithdraw]);
197-202: Improve thehandleEnterfunction.The function has unnecessary type casting and a console.log statement that should be removed before merging to production.
-function handleEnter(event: any) { +function handleEnter(event: KeyboardEvent) { - if (canWithdraw && (event as KeyboardEvent)?.key === 'Enter') { + if (canWithdraw && event.key === 'Enter') { - console.log('WithdrawModal event'); handleWithdraw(); } }src/pages/node/tickets.tsx (1)
95-106: Consider using distinct loading states for the two buttonsBoth action buttons use
redeemAllTicketsFetchingfor their loading state, which means they can't display independent loading states. This could confuse users if both actions are triggered in succession.Consider using
resetTicketStatistics.isFetchingfor the reset button's loading state:- reloading={redeemAllTicketsFetching} + reloading={useAppSelector((store) => store.node.resetTicketStatistics.isFetching)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (40)
config.ts(0 hunks)package.json(1 hunks)src/components/Card/index.tsx(1 hunks)src/components/Code/CodeCopyBox/index.tsx(1 hunks)src/components/ConnectNode/modal.tsx(3 hunks)src/components/Faq/index.tsx(2 hunks)src/components/Modal/node/AddAliasModal.tsx(5 hunks)src/components/Modal/node/FundChannelModal.tsx(5 hunks)src/components/Modal/node/OpenChannelModal.tsx(5 hunks)src/components/Modal/node/OpenMultipleChannelsModal.tsx(4 hunks)src/components/Modal/node/PingModal.tsx(5 hunks)src/components/Modal/node/SendMessageModal.tsx(6 hunks)src/components/Modal/node/WithdrawModal.tsx(7 hunks)src/components/Stepper/index.tsx(1 hunks)src/components/SubpageTitle/index.tsx(1 hunks)src/future-hopr-lib-components/Button/chain-button.jsx(1 hunks)src/future-hopr-lib-components/Button/index.tsx(1 hunks)src/future-hopr-lib-components/Cards/GrayCard.tsx(1 hunks)src/future-hopr-lib-components/Icons/JazzIcon.tsx(1 hunks)src/future-hopr-lib-components/Modal/index.tsx(2 hunks)src/future-hopr-lib-components/Modal/styled.tsx(1 hunks)src/future-hopr-lib-components/NFT/index.jsx(1 hunks)src/future-hopr-lib-components/Navbar/navBar.tsx(1 hunks)src/future-hopr-lib-components/Navbar/navBarItems.tsx(1 hunks)src/future-hopr-lib-components/Navbar/navButton.jsx(1 hunks)src/future-hopr-lib-components/PeerInfo/index.tsx(1 hunks)src/future-hopr-lib-components/Table/table-pro.tsx(1 hunks)src/future-hopr-lib-components/Tooltip/tooltip-fixed-width.tsx(1 hunks)src/hooks/useWatcher/index.ts(1 hunks)src/pages/node/aliases.tsx(1 hunks)src/pages/node/configuration.tsx(5 hunks)src/pages/node/info/index.tsx(1 hunks)src/pages/node/messages.tsx(1 hunks)src/pages/node/metrics.tsx(1 hunks)src/pages/node/ping.tsx(1 hunks)src/pages/node/tickets.tsx(4 hunks)src/store/slices/auth/actionsAsync.ts(1 hunks)src/store/slices/node/actionsAsync.ts(32 hunks)src/store/slices/node/actionsFetching.ts(3 hunks)src/store/slices/node/initialState.ts(2 hunks)
💤 Files with no reviewable changes (1)
- config.ts
🧰 Additional context used
🧬 Code Definitions (13)
src/future-hopr-lib-components/Tooltip/tooltip-fixed-width.tsx (2)
src/future-hopr-lib-components/Button/chain-button.jsx (1) (1)
rest(26-30)src/future-hopr-lib-components/Navbar/navButton.jsx (1) (1)
rest(45-48)
src/future-hopr-lib-components/Button/chain-button.jsx (1)
src/future-hopr-lib-components/Navbar/navButton.jsx (1) (1)
rest(45-48)
src/future-hopr-lib-components/PeerInfo/index.tsx (2)
src/future-hopr-lib-components/Button/chain-button.jsx (1) (1)
rest(26-30)src/future-hopr-lib-components/Navbar/navButton.jsx (1) (1)
rest(45-48)
src/hooks/useWatcher/index.ts (1)
src/store/index.ts (1) (1)
useAppSelector(26-26)
src/pages/node/messages.tsx (1)
src/store/index.ts (1) (1)
useAppSelector(26-26)
src/store/slices/auth/actionsAsync.ts (1)
src/store/index.ts (1) (1)
RootState(20-20)
src/pages/node/info/index.tsx (1)
src/store/index.ts (1) (1)
useAppSelector(26-26)
src/future-hopr-lib-components/Button/index.tsx (2)
src/future-hopr-lib-components/Button/chain-button.jsx (1) (1)
rest(26-30)src/future-hopr-lib-components/Navbar/navButton.jsx (1) (1)
rest(45-48)
src/pages/node/configuration.tsx (1)
src/store/slices/app/index.ts (1) (1)
appActions(118-118)
src/components/Modal/node/WithdrawModal.tsx (1)
src/store/index.ts (1) (1)
useAppSelector(26-26)
src/store/slices/node/actionsAsync.ts (2)
src/store/slices/node/actionsFetching.ts (1) (1)
nodeActionsFetching(24-43)src/store/index.ts (1) (1)
RootState(20-20)
src/pages/node/tickets.tsx (3)
src/store/index.ts (2) (2)
useAppDispatch(25-25)useAppSelector(26-26)src/store/slices/node/actionsAsync.ts (1) (1)
actionsAsync(1685-1719)src/future-hopr-lib-components/Button/IconButton.tsx (1) (1)
IconButton(55-83)
src/components/Modal/node/OpenMultipleChannelsModal.tsx (2)
src/future-hopr-lib-components/Modal/styled.tsx (1) (1)
SDialogContent(25-33)src/future-hopr-lib-components/Navbar/navButton.jsx (1) (1)
Button(44-109)
🔇 Additional comments (88)
src/components/Code/CodeCopyBox/index.tsx (1)
11-15: Improved code readability with multi-line destructuring.The parameter destructuring has been reformatted from a single line to a multi-line format, improving readability without changing any functionality.
src/pages/node/ping.tsx (1)
20-23: Improved readability with multi-line object destructuring.The destructuring of
loginDatahas been reformatted from a single line to a multi-line format, which is consistent with similar formatting changes throughout the codebase.package.json (2)
3-3: Version bump to beta release.The project version has been updated from
2.1.15to2.2.0-beta.0, indicating this PR is preparing for a new beta release.
9-9:Details
❓ Verification inconclusive
SDK dependency update for compatibility with new version.
The
@hoprnet/hopr-sdkdependency has been updated from version2.1.9to2.2.0-beta.4to align with the project's version bump.Let's verify the compatibility between the new SDK version and the application's current implementation:
🏁 Script executed:
#!/bin/bash # Look for any breaking changes or migration-related comments in the codebase rg -i "breaking|migration|upgrade|2\.2\." --type ts --type tsx --type jsxLength of output: 103
Below is an updated verification step. The previous search command failed due to an unrecognized file type error. To re‐verify the absence of any migration notes or breaking changes in TypeScript/TSX/JSX files related to the SDK upgrade, please run this revised script:
#!/bin/bash # Re-run the search for migration or breaking changes related to the SDK upgrade rg -i "breaking|migration|upgrade|2\.2\." -g "*.ts" -g "*.tsx" -g "*.jsx"If no output is produced, it indicates no migration-related comments were found—suggesting that the dependency update to
@hoprnet/hopr-sdk@2.2.0-beta.4is aligned with the version bump. However, please manually verify the script’s output and ensure that no compatibility issues are missed.Action:
- Re-run the above script.
- Confirm that no migration or breaking change messages are present.
- Then approve the dependency update accordingly.
src/future-hopr-lib-components/Button/chain-button.jsx (1)
26-30: Improved readability with multi-line props destructuring.The destructuring of the
propsobject has been reformatted from a single line to a multi-line format, consistent with similar changes in other components (e.g.,navButton.jsx).src/future-hopr-lib-components/Navbar/navBarItems.tsx (1)
29-37: Improved Function Signature FormattingThe updated multi-line parameter formatting for the
NavBarItemscomponent enhances readability and aligns the code with our formatting guidelines. No change in functionality was introduced.src/hooks/useWatcher/index.ts (1)
13-16: Improved Destructuring for API CredentialsChanging the destructuring of
apiEndpointandapiTokento a multi-line format improves clarity and consistency with similar changes elsewhere in the codebase.src/pages/node/aliases.tsx (1)
341-345: Clearer Destructuring inhandleChangeSwitching to a multi-line destructuring for
event.targetin thehandleChangefunction improves clarity without affecting functionality.src/pages/node/messages.tsx (1)
19-22: Refined Destructuring for Auth DataThe updated multi-line destructuring for
apiEndpointandapiTokenfrom the login data enhances the readability of the code. This change is consistent with similar updates in other parts of the codebase.
As a side note, please verify that omitting these variables from the dependency array in the associateduseEffectis intentional.src/future-hopr-lib-components/Icons/JazzIcon.tsx (1)
11-16: Enhanced Readability in Parameter DestructuringThe reformatting of the destructuring in the
JazzIconcomponent—placing each parameter (address,diameter,className, and...rest) on its own line—improves the code’s readability. There are no functional changes, and the component continues to perform as expected.src/future-hopr-lib-components/Navbar/navButton.jsx (1)
45-48: Consistent Multi-Line DestructuringThe updated multi-line destructuring of the
propsobject (extractingsubMenuand the rest of the properties) enhances clarity and aligns with our formatting standards. No functionality has been altered.src/future-hopr-lib-components/Tooltip/tooltip-fixed-width.tsx (1)
9-13: Improved Props Destructuring in Tooltip ComponentExpanding the single-line destructuring into a multi-line format (listing
className,notWide, and...reston separate lines) makes the component easier to read and maintain. The functionality remains unchanged.src/future-hopr-lib-components/Modal/index.tsx (1)
11-14: Enhanced Destructuring in Styled SDialog ComponentReformatting the destructuring of props in the
SDialogstyled component (extractingmaxWidthCssand...rest) into multiple lines improves legibility without affecting behavior.src/future-hopr-lib-components/Navbar/navBar.tsx (1)
17-21: Improved Destructuring in Styled AppBar DefinitionThe refactoring of the
AppBarcomponent’s props destructuring into a multi-line format (listingtallerNavBarOnMobile,webapp, and...rest) significantly enhances readability. This change is purely stylistic and does not affect the component’s behavior.src/pages/node/metrics.tsx (1)
159-162: Enhanced Readability through Multi-line DestructuringBy reformatting the destructuring of
loginDatainto a multi-line structure, the code is now more readable and consistent with similar changes across the codebase. No functional changes are introduced.src/components/SubpageTitle/index.tsx (1)
59-64: Improved Parameter Formatting in Component SignatureTransforming the
SubpageTitlecomponent’s function signature to a multi-line format makes the component's props clearer without modifying its functionality.src/pages/node/info/index.tsx (1)
39-42: Consistent Multi-line Destructuring for Redux StateExpanding the destructuring of
loginDatainto a multi-line format improves clarity and maintains consistency across different pages. The change is purely stylistic while keeping the logic intact.src/future-hopr-lib-components/NFT/index.jsx (1)
60-68: Clearer Function Signature with Multi-line DestructuringThe adjusted multi-line destructuring in the
CustomWidthTooltipcomponent enhances readability without affecting its behavior. The formatting aligns well with similar improvements across components.src/future-hopr-lib-components/Table/table-pro.tsx (1)
58-63: Refactored Pagination Props Destructuring for ClarityChanging the destructuring of props (
count,page,rowsPerPage,onPageChange) in theTablePaginationActionsfunction into a multi-line format significantly improves the readability of the code. This is a purely cosmetic change with no impact on functionality.src/components/Card/index.tsx (1)
56-62: Improved Parameter FormattingThe reformatting of the
Cardcomponent’s parameters into a multi-line destructuring format enhances readability. No functional impact is introduced with this purely stylistic update.src/components/Stepper/index.tsx (1)
154-157: Refactored Function Signature for ClarityThe updated multi-line destructuring in the
getStepStatefunction makes the parameter list (i.e.,indexandcurrentStep) clearer and easier to read. This change is purely cosmetic and does not affect the function’s logic.src/future-hopr-lib-components/Cards/GrayCard.tsx (1)
85-94: Enhanced Parameter Destructuring in GrayCardSplitting the props into separate lines in the
GrayCardcomponent improves code readability and adheres to consistent formatting practices without changing any functionality.src/future-hopr-lib-components/Button/index.tsx (1)
101-111: Improved Props Destructuring in Button ComponentThe multi-line destructuring of the
Buttoncomponent’s props (includingimageOnly,size70,standardWidth, etc.) enhances clarity. The changes are stylistic and have no impact on functionality.src/components/Faq/index.tsx (2)
2-8: Good improvement on code readability.Breaking down Material UI imports into multiple lines makes the code more maintainable and easier to read.
117-121: Function signature formatting improvement.The multi-line destructuring pattern enhances readability and is consistent with the formatting changes across the codebase.
src/components/ConnectNode/modal.tsx (4)
153-153: Good variable extraction for condition logic.Creating the
canConnectToNodevariable makes the code more readable and centralizes the condition logic.
155-160: Well-implemented keyboard event handling.Adding the event listener for keyboard events enhances user experience by allowing keyboard navigation. The cleanup function properly removes the listener when the component unmounts.
369-374: Good implementation of Enter key functionality.The
handleEnterfunction improves user experience by allowing the Enter key to submit the form. The console log statement helps with debugging.
457-457: Consistent use of the new condition variable.Using the
canConnectToNodevariable here maintains consistency with its definition above.src/store/slices/auth/actionsAsync.ts (2)
8-11: Improved import destructuring format.Breaking down the API destructuring into multiple lines improves readability.
17-24: Enhanced function parameter formatting.The multi-line formatting of the thunk parameters and payload destructuring makes the code more readable and maintainable.
src/components/Modal/node/AddAliasModal.tsx (6)
37-43: Excellent consolidation of condition logic.Creating the
canAddAliasvariable centralizes all validation conditions in one place, making the code more maintainable and easier to understand.
45-50: Well-implemented keyboard event handling.Adding the event listener for keyboard events improves user experience. The cleanup function properly removes the listener on unmount.
76-76: Good UX improvement for modal opening.Blurring the active element when opening the modal prevents unintended interactions and improves user experience.
139-144: Good implementation of Enter key functionality.The
handleEnterfunction improves form submission experience by supporting keyboard interaction, consistent with other modals in the application.
189-190: Smart autofocus implementation.The conditional autofocus ensures the empty field receives focus, improving user experience when the modal opens.
Also applies to: 207-208
212-213: Consistent use of the consolidated condition variable.Using the
canAddAliasvariable here maintains consistency with its definition above and simplifies the code.src/components/Modal/node/PingModal.tsx (7)
1-1: Good update to importsAdding the
KeyboardEventtype import is necessary for the new keyboard event handling functionality.
30-30: Good refactoring with descriptive variableCreating a dedicated
canPingvariable improves code readability by clearly expressing the condition for enabling the ping button.
32-37: Good implementation of keyboard event listenerThe useEffect correctly sets up and cleans up the keyboard event listener. This follows React best practices for event handling.
54-54: Improved focus managementBlurring the active element when opening the modal provides a better user experience by removing focus from previously active elements.
119-124: Good addition of keyboard shortcut functionalityThe
handleEnterfunction enhances usability by allowing users to submit the form using the Enter key, which is a common UX pattern.
167-167: Enhanced user experience with autoFocusSetting autoFocus on the input field improves usability by allowing immediate typing after the modal opens.
172-172: Improved code readability with canPing variableUsing the
canPingvariable instead of an inline condition makes the code more maintainable and easier to understand.src/components/Modal/node/OpenMultipleChannelsModal.tsx (7)
1-1: Good update to importsAdding the
useRefimport is necessary for the file input reference implementation.
29-29: Enhanced code readability with descriptive variableCreating a dedicated
canOpenvariable improves code maintainability by centralizing the logic for enabling the action button.
31-36: Well-implemented keyboard event handlingThe useEffect hook properly manages event listener lifecycle, adding it when dependencies change and cleaning it up on unmount.
179-179: Improved focus managementBlurring the active element before opening the file input dialog improves user experience by ensuring clear focus states.
183-188: Enhanced user experience with keyboard shortcutsThe
handleEnterfunction properly implements the Enter key functionality, consistent with other modal components in the application.
244-244: Better user experience with autoFocusAdding
autoFocusto the amount field ensures users can immediately start typing after the CSV is loaded.
250-250: Improved code clarity with canOpen variableUsing the
canOpenvariable instead of repeating the validation logic inline makes the code more maintainable.src/components/Modal/node/OpenChannelModal.tsx (7)
1-1: Good update to importsAdding the
useEffectimport is necessary for the keyboard event handling implementation.
37-37: Enhanced code readability with descriptive variableThe
canOpenvariable centralizes the validation logic, making it reusable and self-documenting.
39-44: Properly implemented keyboard event handlingThe useEffect hook correctly sets up and cleans up event listeners, with appropriate dependencies.
52-52: Improved focus managementBlurring the active element when opening the dialog ensures a clean focus state in the modal.
111-116: Enhanced user experience with keyboard shortcutsThe
handleEnterfunction implementation is consistent with other modals, creating a unified user experience.
158-158: Smart implementation of conditional autoFocusAlternating the
autoFocusbetween fields based on whether peerAddress is already provided improves user workflow.Also applies to: 168-168
174-174: Improved code clarity with canOpen variableUsing the
canOpenvariable for the button's disabled state ensures consistent validation throughout the component.src/components/Modal/node/FundChannelModal.tsx (6)
1-1: Good update to importsAdding the
useEffectimport supports the new keyboard event handling functionality.
31-31: Improved code clarity with canFund variableThe descriptive
canFundvariable makes the validation logic more readable and maintainable.
33-38: Well-implemented event handlingThe useEffect hook properly manages the event listener lifecycle with appropriate dependencies.
41-41: Enhanced focus managementBlurring the active element before opening the dialog ensures a clean focus state transition.
155-155: Smart implementation of conditional autoFocusThe conditional autoFocus implementation intelligently focuses on the right field based on whether channelId is already provided.
Also applies to: 165-165
171-171: Improved code readabilityUsing the
canFundvariable for the button's disabled state makes the code more maintainable and self-documenting.src/pages/node/configuration.tsx (3)
57-63: Good addition of thecanSavevariable for better code clarity.This new boolean condition cleanly encapsulates the logic for determining when the save button should be enabled or disabled, making the code more readable and maintainable.
128-132: Nice refactoring from arrow function to regular function.The function signature change from an async arrow function to a regular function is good for consistency.
235-235: Good use of thecanSavevariable for button enabling/disabling.Using the
canSavevariable to control the disabled state of the button improves both code readability and user experience.src/store/slices/node/actionsFetching.ts (2)
21-22: Good addition of new actions for ticket management.The new actions for redeeming all tickets and resetting ticket statistics are well-named and follow the same pattern as existing actions.
41-42: Properly exported new action creators.The action creators are correctly exported in the
nodeActionsFetchingobject, maintaining consistency with the existing code structure.src/components/Modal/node/SendMessageModal.tsx (2)
232-232: Good UX improvement with blur on modal open.Blurring the active element when opening the modal ensures that keyboard focus is properly managed.
355-355: Good UX improvements with autoFocus and disabled state.Adding autoFocus to the message input when a receiver is selected and disabling the send button based on the
canSendMessagevariable are excellent UX improvements.Also applies to: 403-403
src/components/Modal/node/WithdrawModal.tsx (5)
6-12: Improved import formatting.Breaking the imports into individual lines improves readability and will make future changes easier.
88-91: Good use of destructuring for API credentials.Destructuring
apiEndpointandapiTokenfromloginDatamakes the code cleaner and easier to read.
104-110: Good addition of thecanWithdrawvariable.This variable clearly defines the conditions for when withdrawals are allowed, improving code readability and maintainability.
149-149: Good TODO comment.The TODO comment clearly indicates a future enhancement to update the balances after a withdrawal operation.
288-288: Good use of thecanWithdrawvariable for button enabling/disabling.Using the
canWithdrawvariable to control the disabled state of the button improves both code readability and user experience.src/store/slices/node/initialState.ts (1)
221-224: Renamed state property for better clarityThe rename from
redeemTicketstoredeemAllTicketsprovides better clarity about what this state property represents - the action of redeeming all tickets rather than individual ones.src/pages/node/tickets.tsx (5)
17-19: Icons added for improved UI clarityGood addition of appropriate icons for the ticket management actions.
25-26: State variables renamed for consistencyProperly renamed state variables to match the state property rename in the store.
44-55: Updated function to use renamed thunkThe function now correctly calls the renamed thunk with proper parameters and handles the result.
57-68: Added new function for resetting ticket statisticsGood implementation of the new reset functionality, following the same pattern as other similar functions.
83-94: Updated button for ticket redemptionButton properly using the renamed fetching state.
src/store/slices/node/actionsAsync.ts (6)
81-83: Added new SDK API function importsAdded
redeemAllTicketsandresetTicketStatisticsto the list of imported API functions.
856-881: Properly implemented redeemAllTicketsThunkThe thunk is correctly implemented, following the established patterns. It dispatches the appropriate action to set the loading state and handles errors properly.
883-908: Added new resetTicketStatisticsThunkGood implementation of the new reset functionality as a thunk. It follows the established patterns and correctly dispatches the loading state action.
1331-1346: Updated reducer cases for redeemAllTicketsThunkThe reducer cases have been properly updated to match the renamed state property.
1347-1354: Added reducer case for resetTicketStatisticsThunkThe implementation resets all the relevant ticket statistics values when the thunk is successfully fulfilled. This ensures a clean state for future statistics.
1712-1713: Updated exports to include new thunksProperly added the new and renamed thunks to the exported actionsAsync object.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/node/aliases.tsx (1)
94-103: Improved alias update logic to handle existing mappings.This enhancement checks if a peerId already has an alias and removes it before setting a new one, preventing orphaned or duplicate aliases. This is a good improvement for data consistency.
However, there's a missing trailing comma at the end of line 101 which should be fixed:
apiToken: loginData.apiToken ? loginData.apiToken : '', - }) + }),🧰 Tools
🪛 ESLint
[error] 101-102: Missing trailing comma.
(comma-dangle)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/pages/node/aliases.tsx(3 hunks)src/store/slices/node/actionsAsync.ts(32 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code Definitions (2)
src/pages/node/aliases.tsx (3)
src/store/index.ts (1) (1)
useAppSelector(26-26)src/store/slices/node/actionsAsync.ts (1) (1)
actionsAsync(1685-1719)src/store/slices/auth/actionsAsync.ts (1) (1)
actionsAsync(144-144)
src/store/slices/node/actionsAsync.ts (2)
src/store/slices/node/actionsFetching.ts (1) (1)
nodeActionsFetching(24-43)src/store/index.ts (1) (1)
RootState(20-20)
🪛 ESLint
src/pages/node/aliases.tsx
[error] 101-102: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (9)
src/store/slices/node/actionsAsync.ts (7)
81-82: API update with additional ticket management functions.The API has been updated to include
redeemAllTicketsandresetTicketStatisticsfunctions, which aligns with the new thunks added in this file.
753-753: Incorrect fetching action in redeemChannelTicketsThunkThe
redeemChannelTicketsThunkis usingsetRedeemAllTicketsFetchinginstead of a dedicated action specific to channel tickets. This could lead to unexpected loading state behavior.Consider creating and using a dedicated action for channel ticket redemption:
- dispatch(nodeActionsFetching.setRedeemAllTicketsFetching(true)); + dispatch(nodeActionsFetching.setRedeemChannelTicketsFetching(true));This would require adding the new action in the
actionsFetching.tsfile.
766-767: Inconsistent state reference in redeemChannelTicketsThunk conditionThe condition check for
redeemChannelTicketsThunkis usingstate.redeemAllTickets.isFetchingwhich is inconsistent with the action name and purpose. This could cause unexpected behavior if both redemption processes run simultaneously.Consider using a dedicated state property for channel tickets:
- const isFetching = getState().node.redeemAllTickets.isFetching; + const isFetching = getState().node.redeemChannelTickets.isFetching;This would require adding the new state property in the initialState.
856-881: New thunk for redeeming all tickets.This new thunk provides a dedicated function for redeeming all tickets, which separates this functionality from channel-specific ticket redemption. The implementation follows the established pattern for thunks in this file.
883-908: New thunk for resetting ticket statistics.This adds functionality to reset ticket statistics, which complements the ticket management feature set. The implementation properly handles fetching state and error conditions.
1669-1673: Inconsistent state reference in redeemChannelTicketsThunk handlersThe fulfilled and rejected handlers for
redeemChannelTicketsThunkare settingstate.redeemAllTickets.isFetchingto false. This is inconsistent with the action they're dispatching and could cause state management issues.Consider updating to use a dedicated state property for channel tickets:
- state.redeemAllTickets.isFetching = false; + state.redeemChannelTickets.isFetching = false;This would require adding the new state property in
initialState.tsas well.
1712-1713: Exported newly added thunks.The new thunks have been properly exported in the
actionsAsyncobject, making them available for use throughout the application.src/pages/node/aliases.tsx (2)
37-38: Added utility for mapping peerIds to aliases.The addition of these selectors improves the component's ability to track which peer IDs already have aliases assigned, enabling more robust alias management.
357-360: Improved code readability with multi-line destructuring.The destructuring assignment has been reformatted to a multi-line structure, which improves readability, especially as more properties might be added in the future.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/store/slices/node/actionsAsync.ts (2)
697-697:⚠️ Potential issueIncorrect fetching action in redeemChannelTicketsThunk
The
redeemChannelTicketsThunkis usingsetRedeemAllTicketsFetchinginstead of a dedicated action specific to channel tickets. This could lead to unexpected loading state behavior.Consider creating and using a dedicated action for channel ticket redemption:
- dispatch(nodeActionsFetching.setRedeemAllTicketsFetching(true)); + dispatch(nodeActionsFetching.setRedeemChannelTicketsFetching(true));This would require adding the new action in the
actionsFetching.tsfile.
1623-1627:⚠️ Potential issueInconsistent state reference in redeemChannelTicketsThunk handlers
The fulfilled and rejected handlers for
redeemChannelTicketsThunkare settingstate.redeemAllTickets.isFetchingto false. This is inconsistent with the action they're dispatching and could cause state management issues.Consider updating to use a dedicated state property for channel tickets:
- state.redeemAllTickets.isFetching = false; + state.redeemChannelTickets.isFetching = false;This would require adding the new state property in
initialState.tsas well.
🧹 Nitpick comments (5)
src/components/Modal/node/FundChannelModal.tsx (2)
113-118: Consider adding type safety to the event parameter.The
anytype for the event parameter could be replaced with a more specific type.-function handleEnter(event: any) { +function handleEnter(event: KeyboardEvent) { if (openChannelModal && canFund && event.key === 'Enter') { console.log('FundChannelModal event'); handleAction(); } }
115-115: Remove or improve console.log statement.This console log doesn't provide meaningful information for debugging and should be removed or enhanced with more context.
function handleEnter(event: any) { if (openChannelModal && canFund && event.key === 'Enter') { - console.log('FundChannelModal event'); + // Handled Enter key press in FundChannelModal handleAction(); } }src/components/Modal/node/SendMessageModal.tsx (3)
116-122: Complex validation logic could be simplified.The
canSendMessagelogic is quite complex with multiple nested conditions. Consider breaking it down into smaller, more readable parts.-const canSendMessage = !( - selectedReceiver === null || - (sendMode !== 'directMessage' && sendMode !== 'automaticPath' && numberOfHops < 0 && path === '') || - message.length === 0 || - ((sendMode === 'directMessage' || (sendMode !== 'path' && numberOfHops < 1)) && selectedReceiver === hoprAddress) || - (sendMode === 'path' && path.length === 0) -); +// Check if a receiver is selected +const hasReceiver = selectedReceiver !== null; +// Check if message is not empty +const hasMessage = message.length > 0; +// Check if trying to send to self in direct mode +const isSendingToSelf = selectedReceiver === hoprAddress; +const isDirectToSelf = isSendingToSelf && (sendMode === 'directMessage' || (sendMode !== 'path' && numberOfHops < 1)); +// Check if path mode has a valid path +const hasValidPath = sendMode !== 'path' || path.length > 0; +// Check if routing configuration is valid +const hasValidRouting = sendMode === 'directMessage' || sendMode === 'automaticPath' || + !(numberOfHops < 0 && path === ''); + +const canSendMessage = hasReceiver && hasMessage && !isDirectToSelf && hasValidPath && hasValidRouting;
282-287: Consider handling modifier keys in the Enter key event.The current implementation triggers on any Enter key press. Consider checking for modifier keys to avoid triggering when user is pressing Ctrl+Enter or Shift+Enter to create new lines in the message.
function handleEnter(event: KeyboardEvent) { - if (canSendMessage && (event as KeyboardEvent)?.key === 'Enter') { + if (canSendMessage && event.key === 'Enter' && !event.shiftKey && !event.ctrlKey && !event.altKey) { console.log('SendMessageModal event'); handleSendMessage(); } }
284-284: Remove or improve console.log statement.This console log statement doesn't provide meaningful information for debugging and should be removed or enhanced.
function handleEnter(event: KeyboardEvent) { if (canSendMessage && (event as KeyboardEvent)?.key === 'Enter') { - console.log('SendMessageModal event'); + // Handled Enter key press in SendMessageModal handleSendMessage(); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/components/ConnectNode/modal.tsx(4 hunks)src/components/Modal/node/AddAliasModal.tsx(5 hunks)src/components/Modal/node/FundChannelModal.tsx(5 hunks)src/components/Modal/node/OpenChannelModal.tsx(5 hunks)src/components/Modal/node/OpenMultipleChannelsModal.tsx(4 hunks)src/components/Modal/node/PingModal.tsx(5 hunks)src/components/Modal/node/SendMessageModal.tsx(6 hunks)src/components/Modal/node/WithdrawModal.tsx(6 hunks)src/pages/node/aliases.tsx(2 hunks)src/pages/node/configuration.tsx(5 hunks)src/pages/node/info/index.tsx(5 hunks)src/pages/node/tickets.tsx(5 hunks)src/router.tsx(1 hunks)src/store/slices/node/actionsAsync.ts(15 hunks)src/store/slices/node/initialState.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/pages/node/configuration.tsx
- src/components/Modal/node/PingModal.tsx
- src/components/ConnectNode/modal.tsx
- src/components/Modal/node/OpenMultipleChannelsModal.tsx
- src/pages/node/aliases.tsx
- src/components/Modal/node/WithdrawModal.tsx
- src/components/Modal/node/OpenChannelModal.tsx
- src/components/Modal/node/AddAliasModal.tsx
- src/store/slices/node/initialState.ts
🧰 Additional context used
🧬 Code Definitions (5)
src/router.tsx (1)
src/store/slices/node/index.ts (1)
nodeActionsAsync(64-64)
src/components/Modal/node/FundChannelModal.tsx (2)
src/future-hopr-lib-components/Modal/styled.tsx (1)
SDialogContent(22-30)src/future-hopr-lib-components/Navbar/navButton.jsx (1)
Button(44-106)
src/pages/node/info/index.tsx (1)
src/store/index.ts (1)
useAppSelector(26-26)
src/store/slices/node/actionsAsync.ts (2)
src/store/slices/node/actionsFetching.ts (1)
nodeActionsFetching(24-43)src/store/index.ts (1)
RootState(20-20)
src/pages/node/tickets.tsx (3)
src/store/index.ts (2)
useAppDispatch(25-25)useAppSelector(26-26)src/store/slices/node/actionsAsync.ts (1)
actionsAsync(1648-1683)src/future-hopr-lib-components/Button/IconButton.tsx (1)
IconButton(55-83)
🔇 Additional comments (25)
src/components/Modal/node/FundChannelModal.tsx (5)
31-31: Improved validation logic withcanFundvariable.Centralizing the validation logic in a single variable improves code readability and maintainability.
33-38: Good addition of keyboard event handling.The useEffect hook appropriately sets up and cleans up the keyboard event listener, allowing users to submit the form with the Enter key. The dependency array correctly includes all relevant state variables.
41-41: Improved focus management when opening modal.Blurring the active element before opening the modal prevents focus-related issues and improves user experience.
155-155: Smart focus management withautoFocusproperties.The conditional autoFocus properties improve user experience by focusing on the appropriate input field based on whether the channelId is already populated.
Also applies to: 165-165
171-171: Simplified button disabled logic using thecanFundvariable.Using the centralized
canFundvariable for the button's disabled state improves code readability and consistency.src/components/Modal/node/SendMessageModal.tsx (4)
132-137: Good implementation of keyboard event handling.The useEffect hook properly sets up and cleans up the keyboard event listener. This enhances user experience by allowing message sending with the Enter key.
232-232: Improved focus management when opening the modal.Blurring the active element before opening the modal prevents focus-related issues and improves user experience.
355-355: Good usage of autoFocus for better user experience.The conditional autoFocus property on the message field improves user experience by focusing on the field when a receiver is selected.
403-403: Simplified button disabled logic usingcanSendMessage.Using the centralized
canSendMessagevariable for the button's disabled state improves code readability and consistency.src/router.tsx (1)
326-331: New dispatch added to fetch minimum network probability after loginThis addition enhances the node connection process by retrieving the minimum network probability after a successful login. The implementation is clean and follows the established pattern of other API calls in this useEffect hook.
src/pages/node/info/index.tsx (4)
60-62: Block number variables refactored for clarityThe implementation now uses more descriptive variable names from the Redux store, improving readability and maintainability.
64-64: Added minimum network probability stateAdded state to expose the minimum network probability data from Redux, which aligns with the new functionality introduced throughout this PR.
330-381: Enhanced node information display with improved block dataThe UI has been updated to provide more detailed information about the node's synchronization status, including provider address, current block, last indexed log block, and log checksum. These changes offer users greater visibility into node operations.
463-491: Added ticket properties section with minimum network probabilityNew table section displays ticket information including minimum ticket winning probability, providing valuable context about the network configuration. The tooltip includes dynamic network name from the info object.
src/store/slices/node/actionsAsync.ts (5)
800-822: Renamed redeemTicketsThunk to redeemAllTicketsThunkThe function has been renamed to better reflect its purpose of redeeming all tickets at once. The implementation has been updated consistently with the new naming.
824-846: Added new resetTicketStatisticsThunkNew thunk to reset ticket statistics, enhancing the ticket management functionality. The implementation follows the established pattern for async thunks.
959-985: Added new getMinimumNetworkProbabilityThunkNew thunk to retrieve the minimum network probability from the API. This enhances the available information about network configuration.
1637-1645: Added reducer cases for minimum network probabilityAdded proper handlers for the new getMinimumNetworkProbabilityThunk to update Redux state with the probability data. The implementation follows the established pattern for other similar thunks.
1675-1678: Updated actionsAsync exports with new and renamed thunksThe exported actionsAsync object has been updated to include the new thunks and the renamed thunk, ensuring they're available for use throughout the application.
src/pages/node/tickets.tsx (6)
17-20: Added imports for new UI functionalityAdded icon imports to support the new reset ticket statistics button.
25-30: Updated state variables for consistency with backend changesRenamed variables to match the backend naming changes and added new state variables to access ticket price, network info, and minimum probability data.
47-58: Updated redeemAllTickets handler to use renamed thunkThe function has been updated to call the renamed thunk with the required parameters. The implementation is clean and includes proper error handling.
60-71: Added new handler for resetting ticket statisticsNew function to reset ticket statistics using the new thunk, following the same pattern as other API calls in this component.
86-109: Enhanced UI with new reset statistics buttonAdded a new button to reset ticket statistics and updated the reloading props to reflect the correct loading state. This provides users with more control over their ticket data.
119-147: Added ticket properties display sectionNew table section displays ticket properties including price and minimum winning probability, providing users with important network configuration information. This complements the changes in the info page.
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (2)
src/store/slices/node/actionsAsync.ts (2)
700-700:⚠️ Potential issueIncorrect fetching action in redeemChannelTicketsThunk
The
redeemChannelTicketsThunkis usingsetRedeemAllTicketsFetchinginstead of a dedicated action for channel tickets. This could lead to unexpected loading state behavior.Consider creating and using a dedicated action for channel ticket redemption:
- dispatch(nodeActionsFetching.setRedeemAllTicketsFetching(true)); + dispatch(nodeActionsFetching.setRedeemChannelTicketsFetching(true));This would require adding the new action in the
actionsFetching.tsfile.
1659-1663:⚠️ Potential issueInconsistent state reference in redeemChannelTicketsThunk handlers
The fulfilled and rejected handlers for
redeemChannelTicketsThunkare settingstate.redeemAllTickets.isFetchingto false. This is inconsistent with the action they're dispatching and could cause state management issues.Consider updating to use a dedicated state property for channel tickets:
- state.redeemAllTickets.isFetching = false; + state.redeemChannelTickets.isFetching = false;This would require adding the new state property in
initialState.tsas well.
🧹 Nitpick comments (5)
src/components/ConnectNode/modal.tsx (1)
389-394: Good implementation of the Enter key functionality.The handleEnter function successfully complements the UI button by providing keyboard functionality, improving accessibility and user experience.
Consider removing or conditionally showing the console.log statement in production environments:
function handleEnter(event: KeyboardEvent) { if (props.open && canConnectToNode && event.key === 'Enter') { - console.log('ConnectNode event'); useNode({}); } }src/components/Modal/node/OpenSessionModal.tsx (3)
117-123: Complex condition for canSendMessage could be simplifiedThe condition for
canSendMessageis quite complex and might be difficult to understand and maintain.Consider breaking it down into separate, named conditions for better readability:
- const canSendMessage = !( - selectedReceiver === null || - (sendMode !== 'directMessage' && sendMode !== 'automaticPath' && numberOfHops < 0 && path === '') || - message.length === 0 || - ((sendMode === 'directMessage' || (sendMode !== 'path' && numberOfHops < 1)) && selectedReceiver === hoprAddress) || - (sendMode === 'path' && path.length === 0) - ); + const hasReceiver = selectedReceiver !== null; + const hasValidPath = !(sendMode === 'path' && path.length === 0); + const hasMessage = message.length > 0; + const isNotSendingToSelf = !((sendMode === 'directMessage' || (sendMode !== 'path' && numberOfHops < 1)) && selectedReceiver === hoprAddress); + const hasValidHopsForMode = !(sendMode !== 'directMessage' && sendMode !== 'automaticPath' && numberOfHops < 0 && path === ''); + + const canSendMessage = hasReceiver && hasValidPath && hasMessage && isNotSendingToSelf && hasValidHopsForMode;
161-161: Remove commented out codeThere's a commented line that should be removed as it's no longer needed and may cause confusion.
- //const validatedReceiver = validatePeerId(selectedReceiver);
183-183: Use strict equality comparisonUsing strict equality (
===) is preferred over loose equality (==) in TypeScript/JavaScript.- if (sendMode == 'path') { + if (sendMode === 'path') {src/pages/node/sessions.tsx (1)
347-362: Simplify unnecessary FragmentThe Fragment wrapping the IconButton is unnecessary since there's only one child element.
actions={ - <> <IconButton iconComponent={<GetAppIcon />} tooltipText={ <span> EXPORT <br /> {tabLabel} channels as a CSV </span> } disabled={!channelsData || Object.keys(channelsData).length === 0} onClick={handleExport} /> - </> }🧰 Tools
🪛 Biome (1.9.4)
[error] 348-361: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
package.json(1 hunks)src/components/ConnectNode/modal.tsx(4 hunks)src/components/Modal/node/OpenSessionModal.tsx(1 hunks)src/pages/node/channelsOutgoing.tsx(3 hunks)src/pages/node/sessions.tsx(1 hunks)src/pages/node/tickets.tsx(5 hunks)src/router.tsx(4 hunks)src/store/slices/node/actionsAsync.ts(15 hunks)src/store/slices/node/actionsFetching.ts(3 hunks)src/store/slices/node/initialState.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/store/slices/node/initialState.ts
🧰 Additional context used
🧬 Code Definitions (6)
src/pages/node/channelsOutgoing.tsx (1)
src/components/Modal/node/OpenSessionModal.tsx (1)
OpenSessionModal(98-433)
src/router.tsx (1)
src/store/slices/node/index.ts (1)
nodeActionsAsync(64-64)
src/pages/node/sessions.tsx (1)
src/store/slices/node/actionsAsync.ts (1)
actionsAsync(1693-1729)
src/store/slices/node/actionsAsync.ts (2)
src/store/slices/node/actionsFetching.ts (1)
nodeActionsFetching(26-47)src/store/index.ts (1)
RootState(20-20)
src/pages/node/tickets.tsx (3)
src/store/index.ts (2)
useAppDispatch(25-25)useAppSelector(26-26)src/store/slices/node/actionsAsync.ts (1)
actionsAsync(1693-1729)src/future-hopr-lib-components/Button/IconButton.tsx (1)
IconButton(55-83)
src/components/ConnectNode/modal.tsx (1)
src/store/slices/node/index.ts (1)
nodeActionsAsync(64-64)
🪛 ESLint
src/router.tsx
[error] 347-348: Missing trailing comma.
(comma-dangle)
[error] 354-355: Missing trailing comma.
(comma-dangle)
src/pages/node/sessions.tsx
[error] 58-59: Missing trailing comma.
(comma-dangle)
[error] 65-66: Missing trailing comma.
(comma-dangle)
src/components/ConnectNode/modal.tsx
[error] 332-333: Missing trailing comma.
(comma-dangle)
[error] 339-340: Missing trailing comma.
(comma-dangle)
🪛 Biome (1.9.4)
src/pages/node/sessions.tsx
[error] 348-361: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (27)
src/components/ConnectNode/modal.tsx (3)
153-153: Improving code clarity with a state variable.Creating the
canConnectToNodevariable improves readability by centralizing the button enabling logic in one place.
155-160: Good implementation of keyboard event handling.Adding the keydown event listener with proper cleanup enhances the user experience by allowing Enter key functionality. The dependencies array is comprehensive, ensuring the listener updates when relevant state changes.
477-477: Good use of state variable for button disabled property.Replacing the direct check with the
canConnectToNodevariable improves consistency and makes the code more maintainable.src/pages/node/channelsOutgoing.tsx (2)
25-25: Good addition of OpenSessionModal importThe import is correctly placed with other modal imports, providing a logical organization of the code.
188-189: Appropriately increased width for Actions columnIncreasing the width from '188px' to '225px' accommodates the new OpenSessionModal component that's being added to the actions section.
src/router.tsx (3)
29-29: Good import of SessionsPage componentThe import of the SessionsPage component is properly placed with the other page imports.
58-58: Appropriate icon import for Sessions featureThe SettingsPhoneIcon is a suitable choice for representing sessions functionality.
162-169: Well-structured addition of Sessions navigation itemThe Sessions menu item is correctly added to the NETWORKING section with appropriate properties.
Note: The
numberSessionskey is used, but there doesn't appear to be a corresponding entry in thenumberForDrawerobject. Consider adding this to display session counts in the drawer.src/components/Modal/node/OpenSessionModal.tsx (2)
1-58: Well-structured imports and styled componentsThe imports are organized by category (React, HOPR Components, MUI) and the styled components are well defined with appropriate styles.
69-97: Well-implemented utility functions and typesThe component props interface and the
sortAddressesutility function are well-designed, providing clear types and functionality.src/store/slices/node/actionsFetching.ts (3)
21-24: Proper action additions for ticket-related operationsThese new action creators align well with the expanded functionality for ticket management in the application. They follow the existing naming conventions and action type patterns.
43-46: Export of new action creators follows established patternsThe new action creators are properly exported in the
nodeActionsFetchingobject, maintaining the existing structure and organization of the code.
93-104: Appropriate reducer cases added for new actionsThe new reducer cases correctly update the corresponding state properties when the actions are dispatched. The implementation follows the established pattern in the codebase.
src/pages/node/tickets.tsx (6)
17-19: Appropriate icon imports for new functionalityThe added icon imports support the new UI elements for ticket management operations.
25-30: Redux state selector updates for ticket functionalityState selectors have been updated from
redeemTicketsFetchingtoredeemAllTicketsFetchingto align with the renamed Redux actions. New selectors forinfo,ticketPrice, andminimumNetworkProbabilityhave been added to support the additional ticket properties display.
47-58: Refactored handleRedeemAllTickets implementationThe function has been updated to use the new
redeemAllTicketsThunkand properly unwraps the promise to handle successful redemption.
60-71: New handleResetTicketsStatistics functionThis new function provides the ability to reset ticket statistics, using the newly added
resetTicketStatisticsThunk.
95-96: Updated reloading prop for ticket redemption buttonThe IconButton now correctly references
redeemAllTicketsFetchingto show the loading state during ticket redemption.
179-207: Well-structured table for ticket propertiesThe new table for displaying ticket properties is well-structured and includes helpful tooltips to explain each property to users. It properly displays formatted values and handles null/undefined cases.
src/store/slices/node/actionsAsync.ts (8)
43-46: New type imports for extended functionalityThe type imports have been correctly added to support the new session and probability functionality.
76-87: Integrated new API functionsThe API functions for minimum ticket probability, sessions, and resetting ticket statistics have been properly added to the destructured import list.
803-809: Renamed redeemAllTicketsThunk implementationThe thunk has been properly renamed from
redeemTicketstoredeemAllTicketsto better represent its functionality of redeeming all tickets.
827-853: New resetTicketStatisticsThunk implementationThe new thunk for resetting ticket statistics is well-implemented, following the established patterns for async thunks in the codebase.
966-992: New getMinimumNetworkProbabilityThunk implementationThe implementation of the thunk for fetching minimum network probability follows the codebase's established patterns for async operations.
994-1020: New getSessionsThunk implementationThe implementation of the thunk for fetching sessions follows the codebase's established patterns for async operations.
1673-1690: Properly implemented handlers for new thunksThe fulfilled and rejected handlers for the new thunks (
getMinimumNetworkProbabilityThunkandgetSessionsThunk) are correctly implemented, following the established patterns in the codebase.
1720-1724: Updated actionsAsync export objectThe actionsAsync object has been properly updated to include the new thunks, making them available for use throughout the application.
| dispatch( | ||
| nodeActionsAsync.getSessionsThunk({ | ||
| apiEndpoint, | ||
| apiToken: apiToken ? apiToken : '', | ||
| protocol: 'udp' | ||
| }), | ||
| ) | ||
| dispatch( | ||
| nodeActionsAsync.getSessionsThunk({ | ||
| apiEndpoint, | ||
| apiToken: apiToken ? apiToken : '', | ||
| protocol: 'tcp' | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Missing trailing commas in thunk parameters
The protocol parameter objects are missing trailing commas, which is inconsistent with the code style in the rest of the file.
Apply these fixes:
dispatch(
nodeActionsAsync.getSessionsThunk({
apiEndpoint,
apiToken: apiToken ? apiToken : '',
- protocol: 'udp'
+ protocol: 'udp',
}),
)
dispatch(
nodeActionsAsync.getSessionsThunk({
apiEndpoint,
apiToken: apiToken ? apiToken : '',
- protocol: 'tcp'
+ protocol: 'tcp',
}),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dispatch( | |
| nodeActionsAsync.getSessionsThunk({ | |
| apiEndpoint, | |
| apiToken: apiToken ? apiToken : '', | |
| protocol: 'udp' | |
| }), | |
| ) | |
| dispatch( | |
| nodeActionsAsync.getSessionsThunk({ | |
| apiEndpoint, | |
| apiToken: apiToken ? apiToken : '', | |
| protocol: 'tcp' | |
| }), | |
| ); | |
| dispatch( | |
| nodeActionsAsync.getSessionsThunk({ | |
| apiEndpoint, | |
| apiToken: apiToken ? apiToken : '', | |
| protocol: 'udp', | |
| }), | |
| ) | |
| dispatch( | |
| nodeActionsAsync.getSessionsThunk({ | |
| apiEndpoint, | |
| apiToken: apiToken ? apiToken : '', | |
| protocol: 'tcp', | |
| }), | |
| ); |
🧰 Tools
🪛 ESLint
[error] 347-348: Missing trailing comma.
(comma-dangle)
[error] 354-355: Missing trailing comma.
(comma-dangle)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/components/Modal/node/OpenSessionModal.tsx (4)
23-24: Add a trailing comma after the last import for compliance with ESLint rules.ESLint flagged a missing trailing comma (comma-dangle). This helps maintain cleaner diffs and consistent code style.
-import { - DialogTitle, - DialogActions, - CircularProgress, - TextField, - SelectChangeEvent, - Select, - MenuItem, - Autocomplete, - Tooltip, - IconButton as IconButtonMui -} from '@mui/material'; +import { + DialogTitle, + DialogActions, + CircularProgress, + TextField, + SelectChangeEvent, + Select, + MenuItem, + Autocomplete, + Tooltip, + IconButton as IconButtonMui, +} from '@mui/material';🧰 Tools
🪛 ESLint
[error] 23-24: Missing trailing comma.
(comma-dangle)
154-159: Simplify or extract the complex send condition.The boolean logic to determine
canSendMessageis quite large and may be difficult to maintain. Consider extracting it into a more descriptive function or simplifying the conditions within the component.
284-304: Avoid relying on the older"Spacebar"event key.Modern browsers use
" "or"Space"for the space key. Using"Spacebar"might not work consistently.-if (e.key === ' ' || e.key === 'Spacebar') { +if (e.key === ' ') { e.preventDefault(); ... }
471-472: Add trailing comma in JSX return elements.ESLint flagged a missing trailing comma here. Including the trailing comma in multi-line JSX can help maintain consistency and cleaner diffs.
- </IconButtonMui> - </PathNode> + </IconButtonMui>, + </PathNode>,🧰 Tools
🪛 ESLint
[error] 471-472: Missing trailing comma.
(comma-dangle)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
src/components/Modal/node/OpenSessionModal.tsx(1 hunks)
🧰 Additional context used
🪛 ESLint
src/components/Modal/node/OpenSessionModal.tsx
[error] 23-24: Missing trailing comma.
(comma-dangle)
[error] 471-472: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (2)
src/components/Modal/node/OpenSessionModal.tsx (2)
169-174: Repeat of previous advice: add event listener conditionally and fix dependencies.The component is adding a global keydown listener for the Enter key, potentially conflicting with other parts of the app. Also, the dependency array excludes
canSendMessage,openModal, andhandleSendMessage, which may cause stale closures.Revisit the previous approach:
useEffect(() => { + if (openModal) { window.addEventListener('keydown', handleEnter as EventListener); return () => { window.removeEventListener('keydown', handleEnter as EventListener); }; + } - }, [loginData, message, selectedReceiver, sendMode]); + }, [loginData, message, selectedReceiver, sendMode, canSendMessage, openModal, handleSendMessage]);
388-400: Confirm usage of 'listenHost' and 'sessionTarget'.These state variables are never referenced in
handleSendMessagenor passed elsewhere. Verify whether they're needed for session initialization or can be removed.Do you want to keep them for future expansions, or should they be integrated into the payload? Please confirm.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Modal/node/OpenSessionModal.tsx (2)
167-168: Rename or simplify the double negative variable to improve clarity.
intermediatePathEmptyLinkis set by negating conditions twice, which is confusing. Consider a clearer name and logic, such ashasNullInIntermediatePathorisPathValid, for better readability.
24-25: Add trailing commas to meet ESLint's comma-dangle requirement.The static analysis tool flagged missing trailing commas. Insert them to conform to your linting rules.
IconButton as IconButtonMui -} from '@mui/material'; +}, from '@mui/material'; ... - (elem, index)=>{ + (elem, index)=>{ ... - </PathNode> + </PathNode>,Also applies to: 165-166, 477-478
🧰 Tools
🪛 ESLint
[error] 24-25: Missing trailing comma.
(comma-dangle)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Modal/node/OpenSessionModal.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/components/Modal/node/OpenSessionModal.tsx (2)
src/store/slices/node/initialState.ts (1)
AddressesType(53-53)src/store/slices/node/actionsAsync.ts (1)
actionsAsync(1693-1729)
🪛 ESLint
src/components/Modal/node/OpenSessionModal.tsx
[error] 24-25: Missing trailing comma.
(comma-dangle)
[error] 165-166: Missing trailing comma.
(comma-dangle)
[error] 477-478: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (3)
src/components/Modal/node/OpenSessionModal.tsx (3)
197-202: Conditionally attach the keydown listener only when the modal is open.Attaching global listeners could interfere with other components. This was recommended in a previous review.
221-279: Verify usage of 'listenHost' and 'sessionTarget' in session creation.Although you check both fields to enable the "Open session" button, these values are never forwarded to the
sendMessageThunkpayload. Verify whether you intended to include them in the request body or needed them for a different step.
324-330: Avoid capturing Enter key when the modal is closed.
handleEntertriggers sending even whenopenModalis false, potentially frustrating other parts of the UI. This was also mentioned in a prior review.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/pages/node/aliases.tsx (1)
94-103: Logic improvement for alias uniqueness, but remove debugging code.The logic to remove existing aliases before creating new ones prevents duplicate mappings for the same peerId, which is a good enhancement. However, there's an unnecessary console.log statement that should be removed.
- if (peerIdsWithAliases.includes(data.peerId)) { - console.log(peerIdToAliasLink, data.peerId, peerIdToAliasLink[data.peerId]); + if (peerIdsWithAliases.includes(data.peerId)) { await dispatch( actionsAsync.removeAliasThunk({ alias: peerIdToAliasLink[data.peerId],src/components/Modal/node/OpenSessionModal.tsx (7)
173-175: Remove debugging console log.This console.log statement appears to be leftover debugging code and should be removed.
- useEffect(()=>{ - console.log(intermediatePathError) - }, [intermediatePathError]);
198-198: Remove commented-out code.There's a commented-out line that should be removed.
- //const validatedReceiver = validatePeerId(destination);
214-214: Use strict equality check.Use strict equality (
===) instead of loose equality (==) for type safety.- if (sendMode == 'path') { + if (sendMode === 'path') {
232-232: Remove console log statements.These console.log statements appear to be leftover debugging code and should be removed.
- console.log('@message: ', res?.challenge); handleCloseModal(); }) .catch((e) => { - console.log('@message err:', e); let errMsg = `Sending message failed`;And in the handleEnter function:
if (canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { - console.log('OpenSessionModal event'); handleOpenSession(); }Also applies to: 236-236, 286-286
344-350: Remove commented-out code.There's a chunk of commented-out code that should be removed.
- // renderOption={(option)=>{ - // return ( - // <span> - // {JSON.stringify(option)} - // </span> - // ) - // }}
405-407: Remove commented-out code.There's a commented-out line that should be removed.
<Autocomplete - // key={`path-node-${pathIndex}-Autocomplete`} value={pathNode}
24-25: Fix style issues flagged by ESLint.There are several style issues flagged by ESLint that should be fixed:
- Add missing trailing commas:
IconButton as IconButtonMui - } from '@mui/material'; + } from '@mui/material'; const SFormGroup = styled(FormGroup)` flex-direction: row; - ` + `; const intermediatePathError = fullPath.findIndex( (elem, index)=>{ return elem === fullPath[index+1] - } + }, ) !== -1; </PathNode> - + , )Also applies to: 88-88, 156-157, 437-438
🧰 Tools
🪛 ESLint
[error] 24-25: Missing trailing comma.
(comma-dangle)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Modal/node/OpenSessionModal.tsx(1 hunks)src/pages/node/aliases.tsx(3 hunks)src/pages/node/channelsIncoming.tsx(4 hunks)src/pages/node/channelsOutgoing.tsx(3 hunks)src/pages/node/peers.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/node/channelsOutgoing.tsx
🧰 Additional context used
🧬 Code Definitions (4)
src/pages/node/peers.tsx (1)
src/components/Modal/node/OpenSessionModal.tsx (1)
OpenSessionModal(125-491)
src/pages/node/aliases.tsx (2)
src/store/slices/node/actionsAsync.ts (1)
actionsAsync(1693-1729)src/components/Modal/node/OpenSessionModal.tsx (1)
OpenSessionModal(125-491)
src/pages/node/channelsIncoming.tsx (1)
src/components/Modal/node/OpenSessionModal.tsx (1)
OpenSessionModal(125-491)
src/components/Modal/node/OpenSessionModal.tsx (2)
src/store/slices/node/initialState.ts (1)
AddressesType(53-53)src/store/slices/node/actionsAsync.ts (1)
actionsAsync(1693-1729)
🪛 ESLint
src/components/Modal/node/OpenSessionModal.tsx
[error] 24-25: Missing trailing comma.
(comma-dangle)
[error] 156-157: Missing trailing comma.
(comma-dangle)
[error] 437-438: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (7)
src/pages/node/peers.tsx (2)
20-20: Good addition of the OpenSessionModal component.The OpenSessionModal import enhances the peer interaction functionality by allowing users to initiate sessions directly from the peers list.
189-191: Implementation looks correct.The OpenSessionModal component is correctly implemented with the peerId prop, providing a consistent UX alongside other peer actions like ping, message sending, and channel management.
src/pages/node/channelsIncoming.tsx (3)
26-26: Good addition of the OpenSessionModal component.The OpenSessionModal import enhances the channel interaction functionality by allowing users to initiate sessions directly from the channels list.
142-142: Appropriate UI adjustments for new action button.The column width adjustments are necessary to accommodate the new OpenSessionModal button in the actions column, ensuring proper layout and spacing.
Also applies to: 157-158
315-317: Implementation looks correct.The OpenSessionModal component is correctly implemented with the peerId prop, maintaining consistent UX with other components that use this modal.
src/pages/node/aliases.tsx (2)
30-30: Good state management improvements.The addition of the peerId selector and alias links enhances the component's functionality by providing access to necessary state for alias management and session operations.
Also applies to: 38-39
185-187: Implementation looks correct.The OpenSessionModal component is correctly implemented with the peerId prop, providing consistent UX alongside other peer actions.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (5)
src/pages/node/sessions.tsx (3)
33-33:⚠️ Potential issueComponent name doesn't match file name.
The file is named
sessions.tsxbut exports a component namedChannelsPage. This creates confusion when importing and using the component.- function ChannelsPage() { + function SessionsPage() {And update the export at the end of the file:
- export default ChannelsPage; + export default SessionsPage;
88-146: 🛠️ Refactor suggestionReuse 'headerIncoming' from incoming channels page.
The component defines
headerIncomingbut is focused on sessions. The header structure appears to be copied from the channels page but contains fields that might not be relevant to sessions.Consider creating a session-specific header definition that matches the sessions data structure, or import and adapt the header from a shared location if this is a reusable structure.
358-358:⚠️ Potential issueRemove debugging JSON output.
There's a raw JSON.stringify output of sessions in the JSX, which appears to be debugging code that should be removed before production.
- {JSON.stringify(sessions)}This should be replaced with a proper UI component to display the sessions data in a user-friendly format.
src/components/Modal/node/OpenSessionModal.tsx (1)
192-197: 🛠️ Refactor suggestionImprove keydown event listener management.
The current implementation adds a global keydown event listener without checking if the modal is open, which could interfere with other components. Additionally, the dependencies array is incomplete.
useEffect(() => { - window.addEventListener('keydown', handleEnter as EventListener); - return () => { - window.removeEventListener('keydown', handleEnter as EventListener); - }; - }, [loginData, destination, sendMode]); + if (openModal) { + window.addEventListener('keydown', handleEnter as EventListener); + return () => { + window.removeEventListener('keydown', handleEnter as EventListener); + }; + } + }, [loginData, destination, sendMode, canOpenSession, openModal]);Also update the
handleEnterfunction:function handleEnter(event: KeyboardEvent) { - if (canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { + if (openModal && canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { console.log('OpenSessionModal event'); handleOpenSession(); } }src/store/slices/node/actionsAsync.ts (1)
703-717:redeemChannelTicketsThunkis still wired to the all-tickets fetching flag and selectorThis mirrors the problem called out in the previous review:
dispatch(nodeActionsFetching.setRedeemAllTicketsFetching(true)); … const isFetching = getState().node.redeemAllTickets.isFetching;Coupling the channel-specific operation to the global
redeemAllTicketsstate can mask concurrent operations and lead to confusing UI signals.
Please introduce a dedicatedsetRedeemChannelTicketsFetchingaction and correspondingredeemChannelTicketsslice entry, then use those here (and in the builder handlers).
🧹 Nitpick comments (8)
vite.config.ts (1)
26-32: Add trailing comma for consistency in optimizeDeps.The new optimizeDeps configuration is a good addition for prebundling key UI dependencies during development, which will improve development performance. However, there's a missing trailing comma after the last item in the include array.
optimizeDeps: { include: [ '@emotion/react', '@emotion/styled', - '@mui/material/Tooltip' + '@mui/material/Tooltip', ], },🧰 Tools
🪛 ESLint
[error] 30-31: Missing trailing comma.
(comma-dangle)
src/pages/node/sessions.tsx (2)
55-61: Missing trailing comma in thunk parameters.The parameters object is missing a trailing comma after the apiToken property, which is inconsistent with the code style used throughout the file.
dispatch( actionsAsync.getSessionsThunk({ apiEndpoint, - apiToken: apiToken ? apiToken : '' + apiToken: apiToken ? apiToken : '', }), )🧰 Tools
🪛 ESLint
[error] 58-59: Missing trailing comma.
(comma-dangle)
63-66: Missing trailing comma in function return.The return statement is missing a trailing comma, which is inconsistent with the code style used throughout the file.
const getPeerIdFromPeerAddress = (nodeAddress: string): string => { const peerId = nodeAddressToPeerIdLink[nodeAddress]; - return peerId!; + return peerId!, };src/components/Modal/node/OpenSessionModal.tsx (2)
163-164: Add missing trailing comma.The return statement is missing a trailing comma, which is inconsistent with the code style.
const intermediatePathError = fullPath.findIndex( (elem, index)=>{ - return elem === fullPath[index+1] + return elem === fullPath[index+1], } ) !== -1;🧰 Tools
🪛 ESLint
[error] 163-164: Missing trailing comma.
(comma-dangle)
188-190: Remove debug console log.There's a console.log statement that should be removed before production.
- useEffect(()=>{ - console.log('intermediatePathError', intermediatePathError) - }, [intermediatePathError]);src/pages/node/info/index.tsx (1)
330-340: Enhance provider address tooltip description.The tooltip for the provider address could be more descriptive to better explain its purpose.
<Tooltip - title="The RPC provider address your node uses sync" + title="The RPC provider address your node uses to sync with the blockchain" notWide > <span>Provider address</span> </Tooltip>src/store/slices/node/actionsAsync.ts (2)
969-995: Inconsistent naming between thunk, action type and fetching actionThe thunk is called
getMinimumNetworkProbabilityThunk, yet its action type string is'node/getMinimumTicketProbability'and it dispatchessetMinimumTicketProbabilityFetching.
Pick one terminology (network probability or ticket probability) and use it consistently to avoid confusion when debugging async-action logs.
1256-1270: Early-return guard hides a logic errorThe added guard
if (!peerAddress) return;prevents a crash but also masks a broader issue:
peerAddressshould be mandatory inopenChannelThunk.
Instead of silently returning, validate the payload at the call-site or makepeerAddressnon-optional inOpenChannelPayloadTypeso TypeScript enforces it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
package.json(1 hunks)src/components/ConnectNode/modal.tsx(4 hunks)src/components/Modal/node/OpenSessionModal.tsx(1 hunks)src/future-hopr-lib-components/Section/index.tsx(1 hunks)src/future-hopr-lib-components/Select/index.tsx(1 hunks)src/pages/node/aliases.tsx(4 hunks)src/pages/node/info/index.tsx(7 hunks)src/pages/node/peers.tsx(3 hunks)src/pages/node/sessions.tsx(1 hunks)src/router.tsx(4 hunks)src/store/slices/node/actionsAsync.ts(15 hunks)src/store/slices/node/actionsFetching.ts(3 hunks)vite.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/future-hopr-lib-components/Section/index.tsx
- src/future-hopr-lib-components/Select/index.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/pages/node/peers.tsx
- src/router.tsx
- package.json
- src/components/ConnectNode/modal.tsx
- src/pages/node/aliases.tsx
- src/store/slices/node/actionsFetching.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/pages/node/sessions.tsx (13)
src/store/index.ts (2)
useAppDispatch(25-25)useAppSelector(26-26)src/store/slices/node/actionsAsync.ts (1)
actionsAsync(1723-1760)src/utils/helpers.ts (1)
exportToCsv(6-14)src/hooks/useWatcher/notifications.ts (1)
sendNotification(13-38)config.ts (1)
HOPR_TOKEN_USED(4-4)src/components/Modal/node/PingModal.tsx (1)
PingModal(22-187)src/components/Modal/node/AddAliasModal.tsx (1)
CreateAliasModal(24-226)src/components/Modal/node/FundChannelModal.tsx (1)
FundChannelModal(25-184)src/components/Modal/node/OpenChannelModal.tsx (1)
OpenChannelModal(27-187)src/future-hopr-lib-components/Button/IconButton.tsx (1)
IconButton(55-83)src/components/Modal/node/SendMessageModal.tsx (1)
SendMessageModal(97-432)src/components/SubpageTitle/index.tsx (1)
SubpageTitle(59-87)src/components/Modal/node/OpenSessionModal.tsx (1)
OpenSessionModal(130-525)
🪛 ESLint
vite.config.ts
[error] 30-31: Missing trailing comma.
(comma-dangle)
src/pages/node/sessions.tsx
[error] 58-59: Missing trailing comma.
(comma-dangle)
src/components/Modal/node/OpenSessionModal.tsx
[error] 27-28: Missing trailing comma.
(comma-dangle)
[error] 163-164: Missing trailing comma.
(comma-dangle)
[error] 248-249: Missing trailing comma.
(comma-dangle)
[error] 471-472: Missing trailing comma.
(comma-dangle)
src/store/slices/node/actionsAsync.ts
[error] 1009-1010: Missing trailing comma.
(comma-dangle)
[error] 1013-1014: Missing trailing comma.
(comma-dangle)
[error] 1018-1019: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (5)
src/pages/node/info/index.tsx (4)
60-65: LGTM: New selectors for node information.The addition of new selectors (
blockNumberFromInfo,indexerLastLogBlock,indexerLastLogChecksum, andminimumNetworkProbability) helps improve access to important node metrics.
350-381: LGTM: Updated block number display.The changes to use the new block number selectors for displaying current block, last indexed log block, and log checksum provide better organization and clarity for node indexing status.
464-490: LGTM: Addition of ticket properties table.The new "Ticket properties" table with ticket price and minimum winning probability provides valuable information to users about the network conditions.
729-740: LGTM: Simplified channel counting.The change to display all channels rather than just open ones provides a more complete view of node connections.
src/store/slices/node/actionsAsync.ts (1)
997-1036: Verify thatopenSessionsFetchingupdatesstate.sessions.isFetching
getSessionsThunkstarts withdispatch(nodeActionsFetching.openSessionsFetching(true));but the slice later checks/clears
state.sessions.isFetching.
IfopenSessionsFetchingwrites to a differently-named field (e.g.state.openSessions.isFetching), the flag will be out of sync and the condition guard at lines 1029-1033 will never turnfalse.Could you double-check the corresponding reducer in
actionsFetching.tsor rename the action for clarity?🧰 Tools
🪛 ESLint
[error] 1009-1010: Missing trailing comma.
(comma-dangle)
[error] 1013-1014: Missing trailing comma.
(comma-dangle)
[error] 1018-1019: Missing trailing comma.
(comma-dangle)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/store/slices/node/actionsAsync.ts (1)
1088-1094:⚠️ Potential issue
nodeIsReady.isFetchingnever flips back tofalseIn the fulfilled handler you set the flag to
trueagain, which means subsequent calls will be blocked by theconditionguard.-state.nodeIsReady.isFetching = true; +state.nodeIsReady.isFetching = false;
♻️ Duplicate comments (4)
src/components/Modal/node/OpenSessionModal.tsx (3)
154-156:intermediatePathinitialised with[null]re-introduces previous validation issues
This was flagged in an earlier review and remains unresolved.Initialising with a non-empty array forces users to delete the dummy entry or keeps
canOpenSessiondisabled because of the null value.-const [intermediatePath, set_intermediatePath] = useState<(string|null)[]>([null]); +const [intermediatePath, set_intermediatePath] = useState<(string|null)[]>([]);
194-200: 🛠️ Refactor suggestionGlobal key-down listener fires even when modal is closed
Adding / removing the listener on every render and regardless of
openModalcan lead to duplicate handlers and unexpected “Enter” key reactions elsewhere in the app. React already warned about this pattern in earlier reviews.-useEffect(() => { - window.addEventListener('keydown', handleEnter as EventListener); - return () => { - window.removeEventListener('keydown', handleEnter as EventListener); - }; -}, [loginData, destination, sendMode]); +useEffect(() => { + if (!openModal) return; + window.addEventListener('keydown', handleEnter as EventListener); + return () => window.removeEventListener('keydown', handleEnter as EventListener); +}, [openModal, canOpenSession]); // keep deps minimal & accurate
305-310: 🛠️ Refactor suggestionGuard
handleEnterwithopenModalto avoid side-effectsCoupling the handler with the modal state prevents accidental session creation when the modal is not visible.
-function handleEnter(event: KeyboardEvent) { - if (canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { +function handleEnter(event: KeyboardEvent) { + if (openModal && canOpenSession && (event as KeyboardEvent)?.key === 'Enter') {src/store/slices/node/actionsAsync.ts (1)
705-724: Channel-ticket redemption still toggles the global “all tickets” flagBoth the dispatch and the guard use
redeemAllTickets, contradicting the intent of a channel-specific redemption thunk flagged in the previous review.You need a dedicated
setRedeemChannelTicketsFetchingaction & store slice (redeemChannelTickets) to avoid clashing loading states.
🧹 Nitpick comments (4)
src/hooks/useWatcher/index.ts (1)
109-118: UseintervalDurationparameter or a dedicated constant instead of a hard-coded 20 000All other watcher intervals respect the
intervalDurationprop, making the polling cadence easily adjustable from one place.
Hard-coding20_000fragments this behaviour and complicates future tuning or configuration via env‐vars.- }, 20_000); + }, intervalDuration);If you want sessions to be polled at a different cadence, consider adding a second optional prop (e.g.
sessionsIntervalDuration) rather than inlining a magic number.src/pages/node/sessions.tsx (1)
178-185: Tooltip text still mentions “channels”Minor wording nit – this component exports sessions, not channels.
- EXPORT - <br /> - sessions channels as a CSV + EXPORT + <br /> + sessions as a CSVsrc/store/slices/node/actionsAsync.ts (2)
1011-1023: ESLint: trailing commas & readabilityMinor style fix – keeps diffs cleaner and matches surrounding code.
- ...payload, - protocol: 'tcp' + ...payload, + protocol: 'tcp',Repeat for the UDP call and the
resarray.🧰 Tools
🪛 ESLint
[error] 1011-1012: Missing trailing comma.
(comma-dangle)
[error] 1015-1016: Missing trailing comma.
(comma-dangle)
[error] 1020-1021: Missing trailing comma.
(comma-dangle)
1721-1729:getMinimumNetworkProbabilityThunkreducers omitpendingstate
setMinimumTicketProbabilityFetching(true)is dispatched, but you never reset it to true in thependingphase.
Consider adding:builder.addCase(getMinimumNetworkProbabilityThunk.pending, (state) => { state.probability.isFetching = true; });Otherwise the spinner may flicker because the flag momentarily returns to
falsebetween subsequent calls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(1 hunks)src/components/Modal/node/OpenSessionModal.tsx(1 hunks)src/hooks/useWatcher/index.ts(1 hunks)src/pages/node/sessions.tsx(1 hunks)src/store/slices/node/actionsAsync.ts(15 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/hooks/useWatcher/index.ts (1)
src/store/slices/node/index.ts (1)
nodeActionsAsync(64-64)
🪛 ESLint
src/pages/node/sessions.tsx
[error] 39-40: Missing trailing comma.
(comma-dangle)
src/components/Modal/node/OpenSessionModal.tsx
[error] 27-28: Missing trailing comma.
(comma-dangle)
[error] 165-166: Missing trailing comma.
(comma-dangle)
[error] 509-510: Missing trailing comma.
(comma-dangle)
src/store/slices/node/actionsAsync.ts
[error] 1011-1012: Missing trailing comma.
(comma-dangle)
[error] 1015-1016: Missing trailing comma.
(comma-dangle)
[error] 1020-1021: Missing trailing comma.
(comma-dangle)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/store/slices/node/actionsAsync.ts (3)
758-777:⚠️ Potential issueUnchanged but previously flagged issue in redeemChannelTicketsThunk.
The function is still using
setRedeemAllTicketsFetchingand checkingredeemAllTickets.isFetchinginstead of having its own dedicated state. This could cause issues if both operations run concurrently.
1452-1458:⚠️ Potential issueMissing fetching flag reset in resetTicketStatisticsThunk handlers.
The fulfilled handler doesn't reset the fetching flag, which would leave it stuck in the fetching state.
builder.addCase(resetTicketStatisticsThunk.fulfilled, (state) => { if (!state.statistics.data) return; state.statistics.data.neglectedValue = '0'; state.statistics.data.redeemedValue = '0'; state.statistics.data.rejectedValue = '0'; state.statistics.data.winningCount = 0; + state.resetTicketStatistics.isFetching = false; }); +builder.addCase(resetTicketStatisticsThunk.rejected, (state) => { + state.resetTicketStatistics.isFetching = false; +});
1065-1073:⚠️ Potential issueInconsistent action name in getSessionsThunk.
You're using
openSessionsFetchinginstead of a name that follows the established pattern (setXFetching).- dispatch(nodeActionsFetching.openSessionsFetching(true)); + dispatch(nodeActionsFetching.setSessionsFetching(true));
🧹 Nitpick comments (1)
src/store/slices/node/actionsAsync.ts (1)
1075-1087: Missing trailing commas and inconsistent object structure.The object structure and format is inconsistent with the rest of the codebase.
Apply these changes:
const bothRes = await Promise.all([ getSessions({ ...payload, - protocol: 'tcp' + protocol: 'tcp', }), getSessions({ ...payload, - protocol: 'udp' + protocol: 'udp', }), ]) const res = [ ...bothRes[0], - ...bothRes[1] + ...bothRes[1], ];🧰 Tools
🪛 ESLint
[error] 1077-1078: Missing trailing comma.
(comma-dangle)
[error] 1081-1082: Missing trailing comma.
(comma-dangle)
[error] 1086-1087: Missing trailing comma.
(comma-dangle)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/ConnectNode/modal.tsx(1 hunks)src/components/Modal/node/AddAliasModal.tsx(1 hunks)src/components/Modal/node/WithdrawModal.tsx(2 hunks)src/hooks/useWatcher/index.ts(1 hunks)src/pages/node/aliases.tsx(4 hunks)src/pages/node/configuration.tsx(2 hunks)src/pages/node/info/index.tsx(7 hunks)src/store/slices/node/actionsAsync.ts(8 hunks)src/store/slices/node/actionsFetching.ts(3 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/components/Modal/node/AddAliasModal.tsx
- src/components/Modal/node/WithdrawModal.tsx
- src/pages/node/configuration.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/ConnectNode/modal.tsx
- src/hooks/useWatcher/index.ts
- src/pages/node/aliases.tsx
- src/pages/node/info/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/store/slices/node/actionsAsync.ts (2)
src/store/index.ts (1)
RootState(20-20)src/store/slices/node/actionsFetching.ts (1)
nodeActionsFetching(26-47)
🪛 ESLint
src/store/slices/node/actionsAsync.ts
[error] 1077-1078: Missing trailing comma.
(comma-dangle)
[error] 1081-1082: Missing trailing comma.
(comma-dangle)
[error] 1086-1087: Missing trailing comma.
(comma-dangle)
🔇 Additional comments (10)
src/store/slices/node/actionsFetching.ts (3)
21-21: Looks good: New action for ticket probability fetching.This follows the established naming pattern for fetching actions.
45-46: Exported new actions.Remember to update the action name in the exported object if you rename
openSessionsFetchingtosetSessionsFetching.
96-104: Proper handlers for the new fetching actions.These handlers update the appropriate state properties.
src/store/slices/node/actionsAsync.ts (7)
43-49: Good job adding new types from the SDK.These types properly support the new functionality for sessions and minimum ticket probability.
79-82: New SDK functions correctly imported.These imports support session management and network probability features.
886-912: resetTicketStatisticsThunk implementation looks good.The thunk correctly dispatches the fetching action and handles the API call.
1037-1063: Well implemented getMinimumNetworkProbabilityThunk.This thunk properly manages fetching state and handles API responses.
1787-1795: Good implementation of getMinimumNetworkProbabilityThunk handlers.The fulfilled and rejected handlers properly manage state.
1796-1804: getSessionsThunk handlers properly implemented.The handlers correctly update state and handle API endpoint mismatches.
1837-1840: New async thunks added to exports.These additions make the new functionality available to other parts of the application.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/components/Modal/node/OpenSessionModal.tsx (3)
180-185: 🛠️ Refactor suggestionImprove keydown event listener management.
The current implementation adds a global keydown event listener without checking if the modal is open, which could interfere with other components. Additionally, the dependencies array is incomplete.
useEffect(() => { - window.addEventListener('keydown', handleEnter as EventListener); - return () => { - window.removeEventListener('keydown', handleEnter as EventListener); - }; - }, [loginData, destination, sendMode]); + if (openModal) { + window.addEventListener('keydown', handleEnter as EventListener); + return () => { + window.removeEventListener('keydown', handleEnter as EventListener); + }; + } + }, [loginData, destination, sendMode, canOpenSession, openModal]);Also update the
handleEnterfunction:function handleEnter(event: KeyboardEvent) { - if (canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { + if (openModal && canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { console.log('OpenSessionModal event'); handleOpenSession(); } }
236-251: 🛠️ Refactor suggestionAdd user notification for successful session creation.
The success case silently closes the modal without notifying the user that the session was created successfully.
.then((res) => { console.log('@session: ', res); + sendNotification({ + notificationPayload: { + source: 'node', + name: 'Session opened successfully', + url: null, + timeout: null, + }, + toastPayload: { message: 'Session opened successfully', type: 'success' }, + dispatch, + }); handleCloseModal(); })
309-314: 🛠️ Refactor suggestionRefine handleEnter function to only trigger when modal is open.
The Enter key handler should check if the modal is open to avoid triggering the action when the modal is closed but the key is pressed elsewhere.
function handleEnter(event: KeyboardEvent) { - if (canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { + if (openModal && canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { console.log('OpenSessionModal event'); handleOpenSession(); } }
🧹 Nitpick comments (9)
src/components/Modal/node/OpenSessionModal.tsx (9)
151-151: Initialize intermediatePath as an empty array for better usability.Starting with
[null]as the initial value can cause validation issues and confusion for users. An empty array would be more appropriate since the user hasn't added any path nodes yet.- const [intermediatePath, set_intermediatePath] = useState<(string | null)[]>([null]); + const [intermediatePath, set_intermediatePath] = useState<(string | null)[]>([]);Then adjust the initial display logic at line 467-524 to show an "Add node" button when the array is empty.
176-178: Remove debugging console.log statement.This console.log statement should be removed before production deployment.
- useEffect(() => { - console.log('intermediatePathError', intermediatePathError); - }, [intermediatePathError]);
239-251: Use alias instead of peerId in notification message when available.The notification message uses the raw peer ID, but for better user experience, it should display the alias if one exists.
.then(() => { - const msg = `Session (${protocol}) to ${sessionTarget} is opened`; + const displayName = destination && peerIdToAliasLink[destination] + ? peerIdToAliasLink[destination] + : destination; + const msg = `Session (${protocol}) to ${displayName} is opened`; sendNotification({ notificationPayload: { source: 'node', name: msg, url: null, timeout: null, }, - toastPayload: { message: msg }, + toastPayload: { message: msg, type: 'success' }, dispatch, }); })
420-437: Consider using radio buttons for protocol selection instead of checkboxes.Checkboxes typically represent multiple-choice options, but protocol selection should be mutually exclusive. Using radio buttons (
RadioGroupandRadiofrom MUI) would align better with standard UI patterns.- <span style={{ margin: '0px 0px -2px' }}>Protocol:</span> - <SFormGroup> - <FormControlLabel - control={<Checkbox checked={protocol === 'udp'} />} - label="UDP" - onChange={() => { - set_protocol('udp'); - }} - /> - <FormControlLabel - control={<Checkbox checked={protocol === 'tcp'} />} - label="TCP" - onChange={() => { - set_protocol('tcp'); - }} - /> - </SFormGroup> + <span style={{ margin: '0px 0px -2px' }}>Protocol:</span> + <RadioGroup + row + value={protocol} + onChange={(e) => set_protocol(e.target.value as 'udp' | 'tcp')} + > + <FormControlLabel value="udp" control={<Radio />} label="UDP" /> + <FormControlLabel value="tcp" control={<Radio />} label="TCP" /> + </RadioGroup>Make sure to import the necessary components:
import { RadioGroup, Radio } from '@mui/material';
253-258: Improve error message formatting in user interface.The current implementation concatenates error messages with newline characters, which may not display properly in all browsers. Consider using proper HTML elements for structured presentation.
let errMsg = `Opening session failed`; if (e instanceof sdkApiError && e.hoprdErrorPayload?.status) - errMsg = errMsg + `.\n${e.hoprdErrorPayload.status}`; + errMsg = errMsg + `. ${e.hoprdErrorPayload.status}`; if (e instanceof sdkApiError && e.hoprdErrorPayload?.error) - errMsg = errMsg + `.\n${e.hoprdErrorPayload.error}`; + errMsg = errMsg + `. ${e.hoprdErrorPayload.error}`; set_error(errMsg);Then, in the dialog content:
- <SDialogContent className="error-message">{error}</SDialogContent> + <SDialogContent className="error-message"> + {error.split('. ').map((line, index) => ( + <p key={index}>{line}</p> + ))} + </SDialogContent>
131-132: Add TypeScript documentation comment for the component.Adding JSDoc comments to describe the component purpose, props, and behavior would improve maintainability and help other developers understand how to use it.
+/** + * Modal for opening a new HOPR network session. + * + * This component allows users to configure and establish a new session, specifying: + * - Destination peer + * - Listen host + * - Session target + * - Capabilities (Retransmission, Segmentation) + * - Protocol (UDP, TCP) + * - Path configuration (number of hops or intermediate path) + * + * @param {string} [props.peerId] - Optional peerId to pre-populate the destination field + * @param {boolean} [props.disabled] - Whether the open button should be disabled + * @param {JSX.Element | string} [props.tooltip] - Custom tooltip for the open button + */ export const OpenSessionModal = (props: SendMessageModalProps) => { const dispatch = useAppDispatch();
79-83: Update the type name to match component purpose.The type name
SendMessageModalPropsis confusing since this component is for opening sessions, not sending messages.-type SendMessageModalProps = { +type OpenSessionModalProps = { peerId?: string; disabled?: boolean; tooltip?: JSX.Element | string; };Also update the component parameter type:
-export const OpenSessionModal = (props: SendMessageModalProps) => { +export const OpenSessionModal = (props: OpenSessionModalProps) => {
201-202: Remove commented-out code.Commented-out code should be removed to maintain code cleanliness.
set_loader(true); - //const validatedReceiver = validatePeerId(destination);
369-375: Remove commented-out renderOption function.Commented-out code should be removed to maintain code cleanliness.
- // renderOption={(option)=>{ - // return ( - // <span> - // {JSON.stringify(option)} - // </span> - // ) - // }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
src/components/Card/index.tsx(1 hunks)src/components/Code/CodeCopyBox/index.tsx(1 hunks)src/components/ConnectNode/modal.tsx(2 hunks)src/components/Faq/index.tsx(2 hunks)src/components/Modal/node/AddAliasModal.tsx(2 hunks)src/components/Modal/node/OpenSessionModal.tsx(1 hunks)src/components/Modal/node/WithdrawModal.tsx(4 hunks)src/components/Stepper/index.tsx(1 hunks)src/components/SubpageTitle/index.tsx(1 hunks)src/future-hopr-lib-components/Button/chain-button.jsx(1 hunks)src/future-hopr-lib-components/Button/index.tsx(1 hunks)src/future-hopr-lib-components/Cards/GrayCard.tsx(1 hunks)src/future-hopr-lib-components/Icons/JazzIcon.tsx(1 hunks)src/future-hopr-lib-components/Layout/drawer.tsx(5 hunks)src/future-hopr-lib-components/Layout/index.tsx(3 hunks)src/future-hopr-lib-components/Modal/index.tsx(2 hunks)src/future-hopr-lib-components/Modal/styled.tsx(1 hunks)src/future-hopr-lib-components/NFT/index.jsx(1 hunks)src/future-hopr-lib-components/Navbar/navBar.tsx(1 hunks)src/future-hopr-lib-components/Navbar/navBarItems.tsx(1 hunks)src/future-hopr-lib-components/Navbar/navButton.jsx(1 hunks)src/future-hopr-lib-components/PeerInfo/index.tsx(1 hunks)src/future-hopr-lib-components/Select/index.tsx(2 hunks)src/future-hopr-lib-components/Table/table-pro.tsx(1 hunks)src/future-hopr-lib-components/Tooltip/tooltip-fixed-width.tsx(1 hunks)src/hooks/useWatcher/index.ts(2 hunks)src/pages/node/aliases.tsx(5 hunks)src/pages/node/channelsIncoming.tsx(4 hunks)src/pages/node/channelsOutgoing.tsx(3 hunks)src/pages/node/info/index.tsx(8 hunks)src/pages/node/messages.tsx(1 hunks)src/pages/node/metrics.tsx(1 hunks)src/pages/node/peers.tsx(3 hunks)src/pages/node/ping.tsx(1 hunks)src/pages/node/sessions.tsx(1 hunks)src/router.tsx(9 hunks)src/store/slices/auth/actionsAsync.ts(1 hunks)src/store/slices/node/actionsAsync.ts(31 hunks)src/store/slices/node/actionsFetching.ts(3 hunks)src/store/slices/node/initialState.ts(6 hunks)vite.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (25)
- src/future-hopr-lib-components/PeerInfo/index.tsx
- src/future-hopr-lib-components/Tooltip/tooltip-fixed-width.tsx
- src/future-hopr-lib-components/Modal/styled.tsx
- src/future-hopr-lib-components/Navbar/navButton.jsx
- src/pages/node/ping.tsx
- src/future-hopr-lib-components/Modal/index.tsx
- src/components/SubpageTitle/index.tsx
- src/future-hopr-lib-components/Button/index.tsx
- vite.config.ts
- src/pages/node/messages.tsx
- src/future-hopr-lib-components/Icons/JazzIcon.tsx
- src/components/Card/index.tsx
- src/pages/node/metrics.tsx
- src/future-hopr-lib-components/Layout/index.tsx
- src/store/slices/auth/actionsAsync.ts
- src/future-hopr-lib-components/Navbar/navBarItems.tsx
- src/future-hopr-lib-components/Button/chain-button.jsx
- src/future-hopr-lib-components/Cards/GrayCard.tsx
- src/components/Faq/index.tsx
- src/future-hopr-lib-components/NFT/index.jsx
- src/future-hopr-lib-components/Table/table-pro.tsx
- src/components/Code/CodeCopyBox/index.tsx
- src/components/Stepper/index.tsx
- src/store/slices/node/actionsFetching.ts
- src/components/Modal/node/WithdrawModal.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- src/components/Modal/node/AddAliasModal.tsx
- src/pages/node/channelsOutgoing.tsx
- src/future-hopr-lib-components/Select/index.tsx
- src/hooks/useWatcher/index.ts
- src/pages/node/channelsIncoming.tsx
- src/future-hopr-lib-components/Layout/drawer.tsx
- src/pages/node/peers.tsx
- src/store/slices/node/initialState.ts
- src/pages/node/sessions.tsx
- src/pages/node/info/index.tsx
- src/components/ConnectNode/modal.tsx
- src/store/slices/node/actionsAsync.ts
- src/pages/node/aliases.tsx
- src/router.tsx
🔇 Additional comments (1)
src/future-hopr-lib-components/Navbar/navBar.tsx (1)
17-17: Clean code refactoring!Simplifying the prop destructuring to a single line makes the code more readable while maintaining the same functionality.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/Modal/node/OpenSessionModal.tsx (1)
308-313:⚠️ Potential issueMissing modal state check in handleEnter
The
handleEnterfunction should check if the modal is open before executing logic to prevent unintended actions when the modal is closed.function handleEnter(event: KeyboardEvent) { - if (canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { + if (openModal && canOpenSession && (event as KeyboardEvent)?.key === 'Enter') { console.log('OpenSessionModal event'); handleOpenSession(); } }
🧹 Nitpick comments (14)
.eslintrc.json (2)
19-19: Disabling the jsx-filename-extension ruleThis change turns off the ESLint rule that enforces
.jsxextensions for files containing JSX. While this aligns with the project's new React components (like the Sessions page), completely disabling the rule could lead to inconsistent file naming practices across the codebase.Consider using a more targeted configuration instead of completely disabling the rule:
- "react/jsx-filename-extension": "off", + "react/jsx-filename-extension": ["warn", { "extensions": [".tsx", ".jsx"] }],
22-22: Relaxed unused variables checking could hide potential issuesTurning off the unused variables rule completely might lead to code bloat and make it harder to identify dead code or refactoring opportunities.
Consider using a more targeted configuration instead of completely disabling the rule:
- "@typescript-eslint/no-unused-vars": "off", + "@typescript-eslint/no-unused-vars": ["warn", { + "argsIgnorePattern": "^_", + "varsIgnorePattern": "^_", + "ignoreRestSiblings": true + }],This would still flag unused variables but allow for ignored parameters (prefixed with
_) and destructuring rest patterns.src/components/InfoBar/details.tsx (1)
115-149: Consider refactoring the number formatting function for better maintainabilityThe truncation function has significant duplicated logic and nested conditionals that make it hard to maintain.
Consider refactoring to remove duplication:
const truncateBalanceto5charsWhenNoDecimals = (value: string | number | undefined | null) => { try { if (value && BigInt(value)) { if (typeof value === 'string') value = parseInt(value); - if (BigInt(value) > BigInt(1e9)) { - return '1e9+'; - } else if (BigInt(value) >= BigInt(1e6)) { - const tmp = (value / 1e6).toString(); - if (tmp.includes('.')) { - const [before, after] = tmp.split('.'); - if (before.length === 3) return before + 'm'; - return `${before}.${after.substring(0, 1)}m`; - } else { - if (tmp.length === 3) return `${tmp}3m`; - return `${tmp}.0m`; - } - } else if (BigInt(value) > BigInt(99999)) { - const tmp = (value / 1e3).toString(); - if (tmp.includes('.')) { - const [before, after] = tmp.split('.'); - if (before.length === 3) return before + 'k'; - return `${before}.${after.substring(0, 1)}k`; - } else { - if (tmp.length === 3) return `${tmp}k`; - return `${tmp}.0k`; - } - } + // Handle large numbers with a more maintainable approach + if (BigInt(value) > BigInt(1e9)) { + return '1e9+'; + } + + const suffixes = [ + { threshold: BigInt(1e6), suffix: 'm', divisor: 1e6 }, + { threshold: BigInt(99999), suffix: 'k', divisor: 1e3 } + ]; + + for (const { threshold, suffix, divisor } of suffixes) { + if (BigInt(value) >= threshold) { + const tmp = (value / divisor).toString(); + if (tmp.includes('.')) { + const [before, after] = tmp.split('.'); + if (before.length === 3) return before + suffix; + return `${before}.${after.substring(0, 1)}${suffix}`; + } else { + if (tmp.length === 3) return `${tmp}${suffix}`; + return `${tmp}.0${suffix}`; + } + } + } return value; } } catch (e) { console.warn('Error while paring data to BigInt for InfoBar'); } return value; };src/components/Modal/node/OpenSessionModal.tsx (11)
78-82: Incorrect type name for propsThe type name
SendMessageModalPropsdoesn't align with the component's purpose, which is opening a session, not sending a message.-type SendMessageModalProps = { +type OpenSessionModalProps = { peerId?: string; disabled?: boolean; tooltip?: JSX.Element | string; };
130-130: Prop type mismatchThe component is using
SendMessageModalPropsbut its name isOpenSessionModal.-export const OpenSessionModal = (props: SendMessageModalProps) => { +export const OpenSessionModal = (props: OpenSessionModalProps) => {
368-374: Remove commented-out codeCommented-out code blocks make the codebase harder to read and maintain.
- // renderOption={(option)=>{ - // return ( - // <span> - // {JSON.stringify(option)} - // </span> - // ) - // }}
176-177: Remove debug console logDebug logging statements should be removed from production code.
- useEffect(() => { - console.log('intermediatePathError', intermediatePathError); - }, [intermediatePathError]);
160-161: Negated condition makes the validation logic harder to understandThe double-negation in
intermediatePathEmptyLinkmakes the validation logic harder to read and reason about.- const intermediatePathEmptyLink = !(sendMode === 'path' && !intermediatePath.includes(null)); + const intermediatePathHasEmptyNodes = sendMode === 'path' && intermediatePath.includes(null);Then update the check in
canOpenSession:const canOpenSession = !destinationMissing && !listenHostMissing && !sessionTargetMissing && - ((sendMode === 'path' && !intermediatePathError && !intermediatePathEmptyLink && intermediatePath.length > 0) || + ((sendMode === 'path' && !intermediatePathError && !intermediatePathHasEmptyNodes && intermediatePath.length > 0) || sendMode === 'numberOfHops');
7-7: Duplicated SDK import warningThe SDK is imported twice: once through destructuring and once for types.
-import { utils as hoprdUtils } from '@hoprnet/hopr-sdk'; -import type { GetPeersResponseType, OpenSessionPayloadType } from '@hoprnet/hopr-sdk'; +import { utils as hoprdUtils, type GetPeersResponseType, type OpenSessionPayloadType } from '@hoprnet/hopr-sdk';🧰 Tools
🪛 GitHub Check: Build (22.x)
[warning] 7-7:
'@hoprnet/hopr-sdk' import is duplicated🪛 GitHub Check: Build (20.x)
[warning] 7-7:
'@hoprnet/hopr-sdk' import is duplicated
202-213: Ensure consistency between form values and payloadThe sessionPayload already includes most form values, but capabilities starts as an empty array before conditionally adding values.
Initialize the capabilities array with the selected values directly for better readability:
const sessionPayload: OpenSessionPayloadType = { apiToken: loginData.apiToken ? loginData.apiToken : '', apiEndpoint: loginData.apiEndpoint, destination, listenHost, target: { Plain: sessionTarget, }, - capabilities: [], + capabilities: [ + ...(retransmission ? ['Retransmission'] : []), + ...(segmentation ? ['Segmentation'] : []), + ], protocol, path: {}, };Then you can remove the conditional capability additions at lines 225-230.
200-200: Remove commented codeCommented-out code should be removed before production deployments.
- //const validatedReceiver = validatePeerId(destination);
150-150: Consider empty array as initial stateInitializing
intermediatePathwith[null]forces the user to either fill or delete the first node.- const [intermediatePath, set_intermediatePath] = useState<(string | null)[]>([null]); + const [intermediatePath, set_intermediatePath] = useState<(string | null)[]>([]);This change would require adjusting the add node button to be more prominent when no nodes are added yet.
234-236: Separate notification from action logic for better maintainabilitySuccess handling is split between two
.then()blocks, which makes the flow harder to follow.dispatch(actionsAsync.openSessionThunk(sessionPayload)) .unwrap() .then((res) => { console.log('@session: ', res); - handleCloseModal(); - }) - .then(() => { const msg = `Session (${protocol}) to ${sessionTarget} is opened`; sendNotification({ notificationPayload: { source: 'node', name: msg, url: null, timeout: null, }, toastPayload: { message: msg }, dispatch, }); + handleCloseModal(); })
159-160: Missing early return optimization for path error detectionThe
findIndexcallback should use an early return to improve efficiency.const intermediatePathError = fullPath.findIndex((elem, index) => { - return elem === fullPath[index + 1]; + if (elem && elem === fullPath[index + 1]) return true; + return false; }) !== -1;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.eslintrc.json(1 hunks)src/components/Code/CodeCopyBox/index.tsx(2 hunks)src/components/InfoBar/details.tsx(2 hunks)src/components/Menu/index.tsx(1 hunks)src/components/Modal/node/FundChannelModal.tsx(2 hunks)src/components/Modal/node/OpenMultipleChannelsModal.tsx(2 hunks)src/components/Modal/node/OpenSessionModal.tsx(1 hunks)src/components/Modal/node/SendMessageModal.tsx(3 hunks)src/components/Modal/node/WithdrawModal.tsx(3 hunks)src/hooks/useWatcher/index.ts(1 hunks)src/pages/node/aliases.tsx(4 hunks)src/pages/node/channelsIncoming.tsx(5 hunks)src/pages/node/configuration.tsx(2 hunks)src/pages/node/info/index.tsx(7 hunks)src/store/slices/node/actionsAsync.ts(6 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/components/Menu/index.tsx
- src/components/Modal/node/OpenMultipleChannelsModal.tsx
- src/components/Modal/node/WithdrawModal.tsx
- src/components/Modal/node/FundChannelModal.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- src/hooks/useWatcher/index.ts
- src/pages/node/channelsIncoming.tsx
- src/pages/node/configuration.tsx
- src/pages/node/aliases.tsx
- src/components/Code/CodeCopyBox/index.tsx
- src/pages/node/info/index.tsx
- src/store/slices/node/actionsAsync.ts
🧰 Additional context used
🪛 GitHub Check: Build (22.x)
src/components/Modal/node/OpenSessionModal.tsx
[warning] 7-7:
'@hoprnet/hopr-sdk' import is duplicated
src/components/Modal/node/SendMessageModal.tsx
[warning] 30-30:
'@hoprnet/hopr-sdk' import is duplicated
🪛 GitHub Check: Build (20.x)
src/components/Modal/node/OpenSessionModal.tsx
[warning] 7-7:
'@hoprnet/hopr-sdk' import is duplicated
src/components/Modal/node/SendMessageModal.tsx
[warning] 30-30:
'@hoprnet/hopr-sdk' import is duplicated
🔇 Additional comments (8)
src/components/Modal/node/SendMessageModal.tsx (3)
30-30: Import optimized: Now includes both type definitions.Good update to include both
GetPeersResponseTypeandSendMessagePayloadTypein a single import statement for better organization.Note: The static analysis tool flagging this as a duplicate import is a false positive - these imports serve different purposes (types vs. utils).
🧰 Tools
🪛 GitHub Check: Build (22.x)
[warning] 30-30:
'@hoprnet/hopr-sdk' import is duplicated🪛 GitHub Check: Build (20.x)
[warning] 30-30:
'@hoprnet/hopr-sdk' import is duplicated
132-132: Fixed code indentation.Proper indentation improves code readability and maintains consistent formatting throughout the codebase.
281-281: Improved function parameter formatting.Proper formatting of the
handleEnterfunction parameter enhances code readability and consistency.src/components/InfoBar/details.tsx (2)
119-119: Good improvement to type handlingConverting the string value to a number with
parseInt()before arithmetic operations.
123-123: Improved type safety in numeric operationsCorrectly using value as a number for arithmetic operations after ensuring it's not a string.
Also applies to: 133-133
src/components/Modal/node/OpenSessionModal.tsx (3)
475-477: Simplified state update for intermediatePathCurrent code is using a spread operator correctly to create a new array reference, which is good.
This properly avoids the previous issue of direct state mutation that was identified in a past review.
507-511: Similar good implementation for adding a new path nodeThe code correctly creates a new array reference when adding a node to the path.
500-502: Good implementation: filter method for deleting path nodesUsing the filter method to remove a node is a clean approach that avoids array mutation.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/future-hopr-lib-components/Icons/JazzIcon.tsx (1)
11-11: Optional: Expand props destructuring for clarity.While combining all props on one line is concise, splitting the destructuring across multiple lines can improve readability when more props are added in the future:
- const JazzIcon = ({ address, diameter, className, ...rest }: JazzIconProps) => { + const JazzIcon = ( + { + address, + diameter, + className, + ...rest + }: JazzIconProps + ) => {src/future-hopr-lib-components/Modal/styled.tsx (1)
4-4: Consider using a more specific type instead ofanyThe use of
anytype removes TypeScript's type checking benefits. Consider defining a more specific interface that extends DialogProps to maintain type safety.-export const SDialog = styled(({ maxWidth, ...rest }: any) => <Dialog {...rest} />)` +export const SDialog = styled(({ maxWidth, ...rest }: { maxWidth?: string } & DialogProps) => <Dialog {...rest} />)`🧰 Tools
🪛 GitHub Check: Build (20.x)
[warning] 4-4:
Unexpected any. Specify a different type🪛 GitHub Check: Build (22.x)
[warning] 4-4:
Unexpected any. Specify a different typesrc/future-hopr-lib-components/Modal/index.tsx (1)
39-40: Improve event type and good use of@ts-expect-errorThe change from
@ts-ignoreto@ts-expect-erroris a good practice as it explicitly indicates an expected TypeScript error. However, the event parameter type{}is too generic.-const handleClose = (event: {}) => { +const handleClose = (event: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>) => { // @ts-expect-error onClose(event, 'escapeKeyDown'); };🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.eslintrc.json(2 hunks)src/components/Card/index.tsx(1 hunks)src/components/Code/CodeCopyBox/index.tsx(1 hunks)src/components/Faq/index.tsx(2 hunks)src/components/Modal/node/PingModal.tsx(4 hunks)src/components/Modal/node/WithdrawModal.tsx(4 hunks)src/components/Stepper/index.tsx(1 hunks)src/components/SubpageTitle/index.tsx(1 hunks)src/future-hopr-lib-components/Button/chain-button.jsx(1 hunks)src/future-hopr-lib-components/Button/index.tsx(1 hunks)src/future-hopr-lib-components/Cards/GrayCard.tsx(1 hunks)src/future-hopr-lib-components/Icons/JazzIcon.tsx(1 hunks)src/future-hopr-lib-components/Modal/index.tsx(2 hunks)src/future-hopr-lib-components/Modal/styled.tsx(1 hunks)src/future-hopr-lib-components/NFT/index.jsx(1 hunks)src/future-hopr-lib-components/Navbar/navBar.tsx(1 hunks)src/future-hopr-lib-components/Navbar/navBarItems.tsx(1 hunks)src/future-hopr-lib-components/Navbar/navButton.jsx(1 hunks)src/future-hopr-lib-components/PeerInfo/index.tsx(1 hunks)src/future-hopr-lib-components/Table/table-pro.tsx(2 hunks)src/future-hopr-lib-components/Tooltip/tooltip-fixed-width.tsx(1 hunks)src/hooks/useWatcher/index.ts(2 hunks)src/pages/node/aliases.tsx(5 hunks)src/pages/node/info/index.tsx(8 hunks)src/pages/node/messages.tsx(1 hunks)src/pages/node/metrics.tsx(1 hunks)src/pages/node/ping.tsx(1 hunks)src/store/slices/auth/actionsAsync.ts(1 hunks)src/store/slices/node/actionsAsync.ts(31 hunks)src/utils/date.tsx(1 hunks)src/utils/functions.ts(2 hunks)
✅ Files skipped from review due to trivial changes (19)
- src/components/Card/index.tsx
- src/future-hopr-lib-components/PeerInfo/index.tsx
- src/pages/node/ping.tsx
- src/future-hopr-lib-components/Button/chain-button.jsx
- src/future-hopr-lib-components/Tooltip/tooltip-fixed-width.tsx
- src/future-hopr-lib-components/Button/index.tsx
- src/future-hopr-lib-components/NFT/index.jsx
- src/components/Stepper/index.tsx
- src/pages/node/messages.tsx
- src/future-hopr-lib-components/Navbar/navBar.tsx
- src/pages/node/metrics.tsx
- src/components/Faq/index.tsx
- src/future-hopr-lib-components/Cards/GrayCard.tsx
- src/utils/functions.ts
- src/future-hopr-lib-components/Navbar/navBarItems.tsx
- src/future-hopr-lib-components/Navbar/navButton.jsx
- src/future-hopr-lib-components/Table/table-pro.tsx
- src/components/Modal/node/PingModal.tsx
- src/components/SubpageTitle/index.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components/Code/CodeCopyBox/index.tsx
- src/components/Modal/node/WithdrawModal.tsx
- src/hooks/useWatcher/index.ts
- .eslintrc.json
- src/pages/node/aliases.tsx
- src/store/slices/node/actionsAsync.ts
- src/pages/node/info/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/store/slices/auth/actionsAsync.ts (1)
src/store/index.ts (1)
RootState(20-20)
🪛 GitHub Check: Build (20.x)
src/future-hopr-lib-components/Icons/JazzIcon.tsx
[warning] 3-3:
Include a description after the "@ts-expect-error" directive to explain why the @ts-expect-error is necessary. The description must be 3 characters or longer
src/future-hopr-lib-components/Modal/styled.tsx
[warning] 4-4:
Unexpected any. Specify a different type
🪛 GitHub Check: Build (22.x)
src/future-hopr-lib-components/Icons/JazzIcon.tsx
[warning] 3-3:
Include a description after the "@ts-expect-error" directive to explain why the @ts-expect-error is necessary. The description must be 3 characters or longer
src/future-hopr-lib-components/Modal/styled.tsx
[warning] 4-4:
Unexpected any. Specify a different type
🪛 Biome (1.9.4)
src/future-hopr-lib-components/Modal/index.tsx
[error] 39-39: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Publish
🔇 Additional comments (4)
src/future-hopr-lib-components/Modal/index.tsx (2)
11-11: LGTM! Improved code formattingThe refactored styled component definition improves readability with consistent formatting.
37-37: LGTM! Improved props destructuringThe simplified destructuring enhances readability while maintaining the same functionality.
src/utils/date.tsx (1)
43-43: LGTM! Improved type compatibility with TimeAgo componentConverting the
nowprop to a function matches the expected API of the TimeAgo component. This is a good improvement for type safety and component compatibility.Also applies to: 50-50
src/store/slices/auth/actionsAsync.ts (1)
14-15: LGTM! Good use of destructuring.Extracting
apiEndpointandapiTokenfrom the payload object using destructuring improves code readability and makes the code more maintainable.
esterlus
left a comment
esterlus
left a comment
There was a problem hiding this comment.
Just two general issues, not related to this PR
2 participants
Changelog:
Sessions pagewith action button to close a specifc seesionOpen session modal, which can be triggered from Sessions page or from a specific node on all pagesSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores