Closed ponderingdemocritus closed 1 week ago
The latest updates on your projects. Learn more about Vercel for Git βοΈ
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
eternum | β Ready (Inspect) | Visit Preview | π¬ Add feedback | Jun 23, 2024 10:35am |
β±οΈ Estimated effort to review [1-5] | 3 |
π§ͺ Relevant tests | No |
π Security concerns | No |
β‘ Key issues to review |
Possible Bug: The logic for displaying the resource icons and text in MarketOrderPanel.tsx has been significantly altered. It's important to ensure that these changes do not affect the UI negatively or introduce any inconsistencies, especially with the use of ResourceIcon and text display logic. |
Code Clarity: The changes in useTrade.tsx introduce a new calculation for perLords . This logic is crucial and should be reviewed carefully to ensure it behaves as expected under all conditions. | |
Redundancy: There are commented out sections and potential redundant code in MarketOrderPanel.tsx which could be cleaned up to improve code quality and maintainability. |
Category | Suggestion | Score |
Possible bug |
Add checks to prevent accessing indices of potentially empty arrays___ **Consider adding a check to ensure thattakerGets and makerGets arrays have at least one element before accessing their indices to avoid potential runtime errors.** [client/src/hooks/helpers/useTrade.tsx [90-93]](https://github.com/BibliothecaDAO/eternum/pull/988/files#diff-e42008cf5c4b4bd687484849a10da2429f055707a9335a9c28ac67da60a45f34R90-R93) ```diff perLords: - takerGets[0].resourceId == ResourcesIds.Lords + takerGets.length > 0 && makerGets.length > 0 && takerGets[0].resourceId == ResourcesIds.Lords ? calculateRatio(takerGets, makerGets) : calculateRatio(makerGets, takerGets), ``` Suggestion importance[1-10]: 9Why: This suggestion addresses a potential runtime error by ensuring that the arrays `takerGets` and `makerGets` have elements before accessing their indices, which is crucial for preventing crashes. | 9 |
Add a fallback value for potentially undefined properties___ **Ensure thatfindResourceById is not returning undefined before accessing the trait property to prevent runtime errors.** [client/src/ui/components/trading/MarketOrderPanel.tsx [158-159]](https://github.com/BibliothecaDAO/eternum/pull/988/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR158-R159) ```diff - Suggestion importance[1-10]: 8Why: Adding a fallback value for `findResourceById(resourceId)?.trait` helps prevent runtime errors when the function returns `undefined`, improving the robustness of the code. | 8 | |
Maintainability |
Refactor repeated code into a single reusable component___ **Refactor the repeated use ofResourceIcon with the same parameters into a single component or function to improve code maintainability and reduce redundancy.** [client/src/ui/components/trading/MarketOrderPanel.tsx [384-385]](https://github.com/BibliothecaDAO/eternum/pull/988/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR384-R385) ```diff - Suggestion importance[1-10]: 7Why: This suggestion improves code maintainability by reducing redundancy, making the codebase easier to manage and less error-prone. | 7 |
Enhancement |
Simplify conditional rendering for better readability___ **Replace the ternary operator with a more concise conditional rendering to improve codereadability.** [client/src/ui/components/trading/MarketOrderPanel.tsx [433]](https://github.com/BibliothecaDAO/eternum/pull/988/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR433-R433) ```diff -Create {isBuy ? "Buy " : `Sell `} Order of {resource.toLocaleString()} {findResourceById(resourceId)?.trait} +Create {isBuy ? "Buy" : "Sell"} Order of {resource.toLocaleString()} {findResourceById(resourceId)?.trait} ``` Suggestion importance[1-10]: 5Why: While this suggestion improves readability slightly, the original code is already clear. The improvement is minor and more stylistic. | 5 |
PR Type
enhancement, configuration changes
Description
perLords
ratio inuseTrade
hook.Changes walkthrough π
useTrade.tsx
Enhance trade calculations with `perLords` ratio.
client/src/hooks/helpers/useTrade.tsx
ResourcesIds
import.perLords
calculation to trade object.MarketOrderPanel.tsx
Improve market order panel display and logic.
client/src/ui/components/trading/MarketOrderPanel.tsx
OrderRowHeader
.getsDisplay
andgetDisplayResource
calculations inOrderRow
.OrderCreation
..env.development
Update environment variables for new API endpoints.
client/.env.development
VITE_NETWORK_FEE_TOKEN
.