Add improved player view for live matches - #64
codegen-sh[bot] wants to merge 5 commits into
Conversation
|
Ok this works
|
|
Yes, it works correctly now! The PR includes:
All the requested changes have been implemented and the items column has been removed as requested. The table headers now have a proper hover effect that doesn't make the text disappear. 💻 View my work • React 👍 or 👎 |
There was a problem hiding this comment.
Pull Request Overview
This PR extends the live match player view by adding status indicators, item/ability displays, net worth, and sortable table columns.
- Introduces new data models (
UltimateState,PlayerAbility) and extendsPlayerwith additional fields. - Updates
PlayerTableto support sorting and displays a net worth column. - Enhances
PlayerRowto render status badges, abilities, and styling for dead players.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
PlayerItem.ts |
Adds UltimateState and PlayerAbility interfaces. |
LiveMatchData.ts |
Extends Player interface with optional status fields. |
PlayerTable.tsx |
Implements sorting, new net worth column, and abilities header. |
PlayerTable.scss |
Styles sortable headers and adjusts column widths. |
PlayerRow.tsx |
Renders player status, abilities, and net worth. |
PlayerRow.scss |
Adds styling for status badges and abilities layout. |
Comments suppressed due to low confidence (3)
apps/dota-tv-web/src/models/PlayerItem.ts:1
- [nitpick] The file name
PlayerItem.tscontains bothUltimateStateandPlayerAbility, which is misleading. Consider renaming toPlayerModels.tsor splitting interfaces into their own files.
export interface UltimateState {
apps/dota-tv-web/src/components/Match/PlayerTable.tsx:118
- [nitpick] There’s an empty
<tbody><tr/></tbody>before the data rows—this stub is unnecessary and could be removed to simplify the markup.
<tbody>
apps/dota-tv-web/src/components/Match/PlayerTable.tsx:14
- Sorting logic (
handleSortand sorted output) isn’t covered by tests. Consider adding unit tests for sort state transitions and sorted order.
const handleSort = (column: string) => {
| y: 0, | ||
| is_alive: true, | ||
| respawn_timer: 0, | ||
| has_buyback: true, |
There was a problem hiding this comment.
Defaulting has_buyback to true may misrepresent players who don’t actually have buyback available. It should default to false.
| has_buyback: true, | |
| has_buyback: false, |
| <thead className="table-dark"> | ||
| <tr> | ||
| <th | ||
| onClick={() => handleSort('name')} |
There was a problem hiding this comment.
The sortable <th> elements use role="button" and tabIndex, but lack onKeyDown handlers for Enter/Space. Add keyboard event handling for full accessibility.
| onClick={() => handleSort('name')} | |
| onClick={() => handleSort('name')} | |
| onKeyDown={(event) => handleKeyDown(event, 'name')} |
| const [sortBy, setSortBy] = useState<string>(''); | ||
| const [sortDirection, setSortDirection] = useState<'asc' | 'desc'>('desc'); | ||
|
|
||
| const handleSort = (column: string) => { |
There was a problem hiding this comment.
Using string for sortBy can lead to invalid keys at runtime. Switch to useState<keyof Player> to enforce valid column names.
| const [sortBy, setSortBy] = useState<string>(''); | |
| const [sortDirection, setSortDirection] = useState<'asc' | 'desc'>('desc'); | |
| const handleSort = (column: string) => { | |
| const [sortBy, setSortBy] = useState<keyof Player | null>(null); | |
| const [sortDirection, setSortDirection] = useState<'asc' | 'desc'>('desc'); | |
| const handleSort = (column: keyof Player) => { |
This PR enhances the player view in live matches with the following improvements:
New Features
Visual Improvements
Technical Changes
These changes provide a more comprehensive and interactive view of player information during live matches, making it easier for users to track player status, items, and abilities.
Note: This implementation assumes that the backend will provide the additional player data (items, abilities, etc.). If the backend doesn't currently provide this data, additional work will be needed to integrate with the appropriate data sources.
💻 View my work • About Codegen