-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add user battles tab #2102
base: next
Are you sure you want to change the base?
Changes from 3 commits
0fb3019
23b39ea
2cdc98d
8a18e82
e6d73ff
3dc608d
5306996
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { useUserBattles } from "@/hooks/helpers/battles/useBattles"; | ||
import { BattleListItem } from "../battles/BattleListItem"; | ||
|
||
export const UserBattles = () => { | ||
const battles = useUserBattles(); | ||
|
||
edisontim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return ( | ||
<div className="w-[31rem] py-2 pl-2"> | ||
{battles.length > 0 && ( | ||
<> | ||
<h5>Your battles</h5> | ||
{battles.map((battle) => ( | ||
<BattleListItem key={battle.entity_id} battle={battle} ownArmySelected={undefined} showCompass /> | ||
))} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Sorting battles is recommended for consistent display Looking at the implementation of
🔗 Analysis chainConsider sorting the battles list. According to Issue #2085, there's a requirement for consistent sorting. Consider sorting the battles list before rendering. Let's verify if the 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check if useUserBattles hook implements sorting
# Search for sorting logic in the hook implementation
ast-grep --pattern 'useUserBattles = () => {
$$$
sort($$$)
$$$
}'
# Alternatively, look for any sorting-related code
rg -A 5 'sort\(' "$(fd -t f useBattles.tsx)"
Length of output: 166 Script: #!/bin/bash
# Let's try a broader search for the hook implementation and any sorting logic
# Find the hook implementation
rg -l "useUserBattles" --type typescript
# Once we find the file, let's examine its content
rg -A 10 "useUserBattles" --type typescript
# Also search for any battle-related sorting in the codebase
rg -A 5 "battles?.sort" --type typescript
rg -A 5 "sortBattles" --type typescript
Length of output: 308 Script: #!/bin/bash
# Let's try without specifying the file type
# Find the hook implementation
rg -l "useUserBattles"
# Once we find the file, let's examine its content
rg -A 10 "useUserBattles"
# Also search for any battle-related sorting in the codebase
rg -A 5 "battles?.sort"
rg -A 5 "sortBattles"
# Look for the actual file containing UserBattles component
fd UserBattles
Length of output: 2309 Script: #!/bin/bash
# Let's examine the complete implementation of useBattles hook to understand how battles are processed
# Get the full content of useBattles.tsx
cat client/src/hooks/helpers/battles/useBattles.tsx
# Also look for any timestamp or date fields in Battle type/interface
rg -A 5 "type Battle"
rg -A 5 "interface Battle"
Length of output: 10478 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you sort battles so that they always render in same order @edisontim ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
</> | ||
)} | ||
edisontim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</div> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,26 @@ | ||
import clsx from "clsx"; | ||
import { ReactComponent as Checkmark } from "@/assets/icons/common/checkmark.svg"; | ||
import clsx from "clsx"; | ||
type CheckboxProps = { | ||
enabled: boolean; | ||
onClick?: () => void; | ||
text?: string; | ||
edisontim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
export const Checkbox = ({ enabled }: CheckboxProps) => ( | ||
<div | ||
className={clsx( | ||
"w-3 h-3 flex items-center justify-center rounded-[3px] bg-dark-green-accent border transition-all duration-300 ease-in-out hover:border-white", | ||
enabled ? "border-grey" : "border-gold", | ||
export const Checkbox = ({ enabled, onClick, text }: CheckboxProps) => ( | ||
<div className="flex flex-row justify-center items-center text-center space-x-2"> | ||
<div | ||
onClick={onClick} | ||
className={clsx( | ||
"w-3 h-3 flex items-center justify-center rounded-[3px] bg-dark-green-accent border transition-all duration-300 ease-in-out hover:border-white", | ||
enabled ? "border-grey" : "border-gold", | ||
)} | ||
> | ||
{enabled && <Checkmark className="fill-gold" />} | ||
</div> | ||
{text && ( | ||
<div onClick={onClick} className="text-sm text-gray-300 hover:text-white transition-colors duration-200"> | ||
{text} | ||
</div> | ||
)} | ||
> | ||
edisontim marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{enabled && <Checkmark className="fill-gold" />} | ||
</div> | ||
); |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,18 +2,51 @@ import { useEntities } from "@/hooks/helpers/useEntities"; | |||||||||||||
import { useQuery } from "@/hooks/helpers/useQuery"; | ||||||||||||||
import { EntityArmyList } from "@/ui/components/military/ArmyList"; | ||||||||||||||
import { EntitiesArmyTable } from "@/ui/components/military/EntitiesArmyTable"; | ||||||||||||||
import { UserBattles } from "@/ui/components/military/UserBattles"; | ||||||||||||||
import { Tabs } from "@/ui/elements/tab"; | ||||||||||||||
import { ID } from "@bibliothecadao/eternum"; | ||||||||||||||
import { useState } from "react"; | ||||||||||||||
|
||||||||||||||
export const Military = ({ entityId, className }: { entityId: ID | undefined; className?: string }) => { | ||||||||||||||
const { isMapView } = useQuery(); | ||||||||||||||
|
||||||||||||||
const { playerStructures } = useEntities(); | ||||||||||||||
|
||||||||||||||
const selectedStructure = playerStructures().find((structure) => structure.entity_id === entityId); | ||||||||||||||
|
||||||||||||||
const [selectedTab, setSelectedTab] = useState(0); | ||||||||||||||
|
||||||||||||||
const tabs = [ | ||||||||||||||
edisontim marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
{ | ||||||||||||||
label: "Army", | ||||||||||||||
component: isMapView ? ( | ||||||||||||||
<EntitiesArmyTable /> | ||||||||||||||
) : ( | ||||||||||||||
selectedStructure && <EntityArmyList structure={selectedStructure} /> | ||||||||||||||
), | ||||||||||||||
}, | ||||||||||||||
{ label: "Battles", component: <UserBattles /> }, | ||||||||||||||
]; | ||||||||||||||
|
||||||||||||||
return ( | ||||||||||||||
<div className={`relative ${className}`}> | ||||||||||||||
{isMapView ? <EntitiesArmyTable /> : selectedStructure && <EntityArmyList structure={selectedStructure} />} | ||||||||||||||
<Tabs | ||||||||||||||
selectedIndex={selectedTab} | ||||||||||||||
onChange={(index: any) => { | ||||||||||||||
setSelectedTab(index); | ||||||||||||||
}} | ||||||||||||||
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Replace 'any' type with a specific type for onChange handler. The onChange handler uses 'any' type which reduces type safety. Consider using a more specific type. -onChange={(index: any) => {
+onChange={(index: number) => {
setSelectedTab(index);
}} 📝 Committable suggestion
Suggested change
|
||||||||||||||
className="h-full" | ||||||||||||||
> | ||||||||||||||
<Tabs.List> | ||||||||||||||
{tabs.map((tab, index) => ( | ||||||||||||||
<Tabs.Tab key={index}>{tab.label}</Tabs.Tab> | ||||||||||||||
))} | ||||||||||||||
</Tabs.List> | ||||||||||||||
|
||||||||||||||
<Tabs.Panels className="overflow-hidden"> | ||||||||||||||
{tabs.map((tab, index) => ( | ||||||||||||||
<Tabs.Panel key={index}>{tab.component}</Tabs.Panel> | ||||||||||||||
))} | ||||||||||||||
</Tabs.Panels> | ||||||||||||||
</Tabs> | ||||||||||||||
</div> | ||||||||||||||
); | ||||||||||||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be memod?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id say so as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, added that on my commit