Closed edisontim 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 25, 2024 6:28pm |
β±οΈ Estimated effort to review [1-5] | 3 |
π§ͺ Relevant tests | No |
π Security concerns | No |
β‘ Key issues to review |
Possible Bug: The logic change in Battles.tsx for showBattleLabel now depends on selectedEntity being undefined, which might not be the intended behavior. Ensure this logic correctly reflects the requirements. |
Code Consistency: In BattleActions.tsx , the isClaimable logic now includes a check for !structure.isMine . Ensure that this new condition is consistent with the application's rules for claiming structures. | |
Styling Consistency: Several files have changes in class names and styles. Verify that these changes do not break the UI's consistency across different browsers and devices. |
Category | Suggestion | Score |
Maintainability |
Simplify the nested ternary operation for better readability___ **The ternary operation for setting thename variable is nested and might be confusing. Consider simplifying it for better readability and to avoid potential bugs.** [client/src/hooks/helpers/useEntities.tsx [127-128]](https://github.com/BibliothecaDAO/eternum/pull/1014/files#diff-655bd94eaba19bfcec06d7b736d2e64944c31d55f2b38819201a2b0b612787e4R127-R128) ```diff -? `${structure?.category} ${structureName}` -: structure?.category; +structureName ? `${structure?.category} ${structureName}` : structure?.category; ``` Suggestion importance[1-10]: 8Why: The suggestion improves readability and maintainability by simplifying the nested ternary operation, which can be confusing and prone to errors. | 8 |
Remove unnecessary space character for cleaner code___ **The loading spinner div has a redundant space character after the closing tag of the innerdiv. This can be removed to clean up the code.** [client/src/ui/elements/Button.tsx [68]](https://github.com/BibliothecaDAO/eternum/pull/1014/files#diff-f1b4f3490361d33c7442ecd46323e03c85ce83161c56dbc76061533ec1476b6cR68-R68) ```diff -{" "} + ``` Suggestion importance[1-10]: 5Why: The suggestion removes an unnecessary space character, which is a minor improvement for code cleanliness and maintainability. | 5 | |
Enhancement |
Remove redundant condition check for clarity and efficiency___ **The condition forshowBattleLabel includes a redundant check for selectedEntity === undefined . Since selectedBattle is also checked for being not undefined, the first check can be removed.** [client/src/ui/components/models/buildings/worldmap/Battles.tsx [43]](https://github.com/BibliothecaDAO/eternum/pull/1014/files#diff-5f342f04015b76da66b7e97f1c3491940764ce97259e444fa972a6db2667517cR43-R43) ```diff -return selectedEntity === undefined && selectedBattle !== undefined && selectedBattle.id === BigInt(battle_id); +return selectedBattle !== undefined && selectedBattle.id === BigInt(battle_id); ``` Suggestion importance[1-10]: 7Why: Removing the redundant check for `selectedEntity === undefined` simplifies the condition and improves code clarity without changing functionality. | 7 |
Possible issue |
Verify and potentially adjust the extended condition for claiming to ensure correct logic___ **TheisClaimable condition has been extended with additional checks. Ensure that the added condition !structure!.isMine is necessary and correctly reflects the intended logic, as it might restrict the claimability more than intended.** [client/src/ui/modules/military/battle-view/BattleActions.tsx [140]](https://github.com/BibliothecaDAO/eternum/pull/1014/files#diff-a2b38e5f9c040c3591adb945a143c3fe234beb433a77a677a8d069ea47a6a462R140-R140) ```diff -Boolean(ownArmy) && !isActive && defenceIsEmptyOrDead && !isRealm && Boolean(structure) && !structure!.isMine; +Boolean(ownArmy) && !isActive && defenceIsEmptyOrDead && !isRealm && Boolean(structure) && (!structure!.isMine || someOtherCondition); ``` Suggestion importance[1-10]: 6Why: The suggestion highlights a potential issue with the extended condition for `isClaimable`. It prompts a review to ensure the logic is correct, which is important for functionality. | 6 |
User description
PR Type
enhancement, bug fix
Description
useEntities.tsx
.selectedEntity
inBattles.tsx
.MarketOrderPanel.tsx
.Button.tsx
.Battle.tsx
.structure.isMine
inBattleActions.tsx
.BattleDetails.tsx
.BattleSideView.tsx
.EntityAvatar.tsx
.LockedResources.tsx
.Troops.tsx
.Changes walkthrough π
11 files
useEntities.tsx
Fix import order and adjust indentation.
client/src/hooks/helpers/useEntities.tsx
shortString
.Battles.tsx
Update battle label logic with selected entity.
client/src/ui/components/models/buildings/worldmap/Battles.tsx
selectedEntity
touseMemo
dependencies.showBattleLabel
logic to includeselectedEntity
.MarketOrderPanel.tsx
Adjust button size and height in MarketOrderPanel.
client/src/ui/components/trading/MarketOrderPanel.tsx - Adjusted button size and height.
Button.tsx
Fix button styles and loading spinner alignment.
client/src/ui/elements/Button.tsx
xs
size class.Battle.tsx
Update button text and adjust battle view height.
client/src/ui/modules/military/battle-view/Battle.tsx
BattleDetails.tsx
Adjust layout and spacing in BattleDetails.
client/src/ui/modules/military/battle-view/BattleDetails.tsx
BattleSideView.tsx
Improve layout and alignment in BattleSideView.
client/src/ui/modules/military/battle-view/BattleSideView.tsx
EntityAvatar
and battle join logic in a flex container.BattleView.tsx
Improve readability with adjusted indentation.
client/src/ui/modules/military/battle-view/BattleView.tsx - Adjusted indentation for better readability.
EntityAvatar.tsx
Add margin and ensure minimum width for EntityAvatar.
client/src/ui/modules/military/battle-view/EntityAvatar.tsx
LockedResources.tsx
Update layout and alignment in LockedResources.
client/src/ui/modules/military/battle-view/LockedResources.tsx - Updated layout and alignment of locked resources container.
Troops.tsx
Adjust layout and remove whitespace in Troops.
client/src/ui/modules/military/battle-view/Troops.tsx
1 files
BattleActions.tsx
Add ownership check for claim logic and disable raid button.
client/src/ui/modules/military/battle-view/BattleActions.tsx
structure.isMine
in claim logic.structure.isMine
.