refactor(CharacterStats): CharacterStats to Table2 and widget3 - #7967
Conversation
steve020607
changed the title
There was a problem hiding this comment.
Pull request overview
Migrates the CharacterStats table rendering from the legacy DataTable/Html.Table approach to the newer Table2 widget system, aligning CharacterStats with the Table2 component API used elsewhere in the commons widget layer.
Changes:
- Replaced
DataTableusage inCharacterStats/Table.luawithTable2(TableWidgets.Table,TableHeader,TableBody,Row,Cell,CellHeader). - Replaced the unchosen-characters
DataTableinCharacterStats.luawithTable2.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lua/wikis/commons/Widget/CharacterStats/Table.lua | Converts the main stats table and details tables to Table2 components (header/body/rows/cells). |
| lua/wikis/commons/Widget/CharacterStats.lua | Converts the “unchosen characters” table to Table2. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
hjpalpha
left a comment
hjpalpha
left a comment
There was a problem hiding this comment.
should convert from widget v2 to widget v3 too
on the same pr? |
|
will be quite a lot of commit, since i do it each one of it :p |
|
the rest of the luals error seems like i'm just don't know what it is |
hjpalpha
left a comment
hjpalpha
left a comment
There was a problem hiding this comment.
why did you adjust the non widget stuff too?
that would need a refactor and when doing so should move into the new structure we want for features
So only widget characterstats and widget characterstats table? |
hjpalpha
left a comment
hjpalpha
left a comment
There was a problem hiding this comment.
skimmed through it, seems okay
steve020607
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
lua/wikis/commons/Widget/CharacterStats/Table.lua:349
- The "complete statistics" link row uses a hard-coded
colspan = 22, but the actual column count varies with#props.sidesand whether bans/global bans are included (e.g., Brawl Stars has 0 sides; LoL/HOK have 2;includeGlobalBansdefaults to false). A mismatch can break table layout/sorting by creating extra implicit columns.
colspan = 22,
Co-authored-by: Rikard Blixt <rikardblixt@gmail.com>
* refactor: CharacterStats to Table2 * Suggestion by @Copilot * all of the checker will not liked this commit :p * As claude suggest * as me and claude says * LINT * huhhhh, idk why the commit just doing that * well * unused import(lua style) * well misunderstood it :p, revert non widget module * Add anno as suggestion * chore: update visual snapshots * suggestion * correcting anno and remove diagnostic * chore: update visual snapshots * use nil instead of bool * suggestion * suggestion Co-authored-by: Rikard Blixt <rikardblixt@gmail.com> --------- Co-authored-by: steve020607 <172269042+steve020607@users.noreply.github.com> Co-authored-by: hjpalpha <75081997+hjpalpha@users.noreply.github.com> Co-authored-by: Rikard Blixt <rikardblixt@gmail.com>
Summary
CharacterStats to Table2
How did you test this change?
Dev
https://liquipedia.net/leagueoflegends/User:Steve23/Testing_Page/1
https://liquipedia.net/brawlstars/User:Steve23