Skip to content

feat(PrizePool): redesign table via Table2 with collapse rework#7688

Open
Eetwalt wants to merge 35 commits into
mainfrom
pp-redesign
Open

feat(PrizePool): redesign table via Table2 with collapse rework#7688
Eetwalt wants to merge 35 commits into
mainfrom
pp-redesign

Conversation

@Eetwalt

@Eetwalt Eetwalt commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Migrate prize pool table rendering from the legacy Widget/Table to Widget/Table2 (header/body/footer sections, declarative rowspan/colspan)
  • Reorder columns: Participant moves directly after Place; points column now renders before qualifies; currency/points cells right-aligned
  • Rework collapse: class-based general-collapsible toggle rendered in the footer, replacing the per-row applyToggleExpand machinery; child classes now only supply open/close label text
  • Rewrite per-column prize-cell merging via a prize matrix + Array.groupAdjacentBy/Table.deepEquals, extracted into _opponentPrizeCells
  • Display placements as plain numbers/ranges (e.g. 1, 1-2) instead of ordinals, with colored gold/silver/bronze badges and pale row tints for top-3
  • Restyle stylesheet for the Table2 structure; skip the legacy JS toggle when the table is inside .prizepool-table-wrapper
  • Emit rowspan/colspan from Table2 Cell/CellHeader even without column definitions

How did you test this change?

dev + devtools

Before After
image image
image image

@Eetwalt Eetwalt requested a review from a team as a code owner June 23, 2026 09:30
Copilot AI review requested due to automatic review settings June 23, 2026 09:30
@Eetwalt Eetwalt requested a review from a team as a code owner June 23, 2026 09:30
@Eetwalt Eetwalt added c: standard c: prize_pool stylesheets Changes to stylesheets javascript Changes to JavaScript files labels Jun 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes prize pool table rendering by migrating from the legacy table widget to Table2, updating column order/alignment, and replacing the old per-row collapse mechanism with a general-collapsible footer toggle.

Changes:

  • Migrate Module:PrizePool/* table output to Table2 (header/body/footer) with updated column ordering and alignment.
  • Rework collapse behavior to use general-collapsible + CSS-hidden rows instead of legacy injected toggle rows (and skip the legacy JS toggle for the new wrapper).
  • Add top-3 placement badge/tint styling and switch place display from ordinals to numeric ranges.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
stylesheets/commons/Prizepooltable.scss Restyles prize pool tables for Table2 structure, new top-3 visuals, and wrapper-based collapse behavior.
lua/wikis/commons/Widget/Table2/CellHeader.lua Ensures rowspan/colspan attributes are emitted even without column definitions.
lua/wikis/commons/Widget/Table2/Cell.lua Ensures rowspan/colspan attributes are emitted even without column definitions.
lua/wikis/commons/PrizePool/Placement.lua Switches place display to numeric/ranges and adds badge class helper for top-3.
lua/wikis/commons/PrizePool/Base.lua Core migration to Table2, new collapse toggle footer, and new prize-cell matrix merging logic.
lua/wikis/commons/PrizePool/Award.lua Updates award table to Table2 and adopts new collapse label mechanism.
lua/wikis/commons/PrizePool.lua Updates placement prize pool rendering (badges, collapse labels) and removes legacy toggle logic.
lua/wikis/commons/Placement.lua Removes the 4th-place background class mapping.
lua/spec/prize_pool_spec.lua Updates test expectations to match new prize column sort order.
javascript/commons/Prizepooltable.js Skips legacy toggle injection for the redesigned Table2 prize pool wrapper to avoid duplicate toggles.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lua/wikis/commons/PrizePool/Base.lua
Comment thread lua/wikis/commons/PrizePool/Base.lua
Comment on lines +731 to +735
local cell = reward and prizeTypeData.rowDisplay(prize.data, reward) or TableCell{}

previousPlacement = placement
end
local lastCellOfType = previousOfPrizeType[prize.type]
if lastCellOfType and prizeTypeData.mergeDisplayColumns then
if Table.isNotEmpty(lastCellOfType.props.children) and Table.isNotEmpty(cell.props.children) then

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cell.props.children is never nil but empty table, so Table.isNotEmpty handles that.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use Logic.isNotEmpty for more flexibility just in case?

@ElectricalBoy ElectricalBoy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from a quick glance on phone

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am somewhat confident that we cannot kick the existing styles (blame smash)

@Eetwalt Eetwalt Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah.. Probably so. Reverted with some comments

Comment thread lua/wikis/commons/PrizePool/Base.lua
Comment thread lua/wikis/commons/PrizePool/Base.lua Outdated

@ElectricalBoy ElectricalBoy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • what implications does this pose to the overall memory/runtime? (was one of my concerns when I was writing #7687)
  • since we are at it this should also take room for currency switch into account (see #7687)

Comment on lines -725 to 777
---@protected
--- Whether a placement's rows are hidden behind the collapse toggle.
--- Child classes override this; the default keeps every row visible.
---@param placement BasePlacement
---@return boolean
function BasePrizePool:applyCutAfter(placement)
error('Function applyCutAfter needs to be implemented by a child class of "Module:PrizePool/Base"')
return false
end

---@protected
---@param placement BasePlacement?
---@param nextPlacement BasePlacement
---@param row WidgetTableRow
function BasePrizePool:applyToggleExpand(placement, nextPlacement, row)
error('Function applyToggleExpand needs to be implemented by a child class of "Module:PrizePool/Base"')
---Open/close labels for the collapse toggle. Child classes override.
---@return {opentext: string, closetext: string}?
function BasePrizePool:_collapseText()
return nil
end

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should keep @protected annotations

Comment on lines +335 to +344
---Returns the placement-badge color class for top-3 placements, else nil.
---Colored by the top of the range (placeStart).
---@return string?
function Placement:getBadgeClass()
if self:hasSpecialStatus() then
return
end
local badges = {[1] = 'placement-1', [2] = 'placement-2', [3] = 'placement-3'}
return badges[self.placeStart]
end

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should make use of Module:Placement

Comment thread stylesheets/commons/Prizepooltable.scss Outdated
Comment on lines +367 to +369
&::-webkit-scrollbar {
display: none;
}

@ElectricalBoy ElectricalBoy Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somewhat redundant as we already have scrollbar-width: none (tbf scrollbar-width itself is baseline newly available)
maybe hide this behind @supports not (scrollbar-width: none)?

Comment thread stylesheets/commons/Prizepooltable.scss Outdated
Comment on lines +433 to +437
// The theme wrapper + `.table2__table` qualifier raise specificity above the
// Table2 zebra rule so the tint wins regardless of load order; `:hover` keeps it.
// `border-bottom: 0` drops the legacy per-place coloured row border.
.theme--light &.prizepooltable-placement.table2__table,
.theme--dark &.prizepooltable-placement.table2__table {

@ElectricalBoy ElectricalBoy Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe lower table2 zebra rules' specificities with :where() instead?
not a fan of having a lot of table2-specific selectors here

Comment thread lua/wikis/commons/PrizePool/Base.lua Outdated
Comment on lines +698 to +701
children = {tostring(OpponentDisplay.BlockOpponent{
opponent = opponent.opponentData,
showPlayerTeam = true,
})},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tostring is redundant

Comment thread lua/wikis/commons/PrizePool/Base.lua Outdated
Comment on lines +623 to +632
children = {
Span{
classes = {'general-collapsible-expand-button'},
children = {Span{children = {collapseText.opentext, ' ', IconFa{iconName = 'expand'}}}},
},
css = {width = 'max-content'},
columns = headerRow:getCellCount(),
children = WidgetUtil.collect(headerRow, unpack(self:_buildRows()))
}},
Span{
classes = {'general-collapsible-collapse-button'},
children = {Span{children = {collapseText.closetext, ' ', IconFa{iconName = 'collapse'}}}},
},
},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could (should?) use Module:Widget/GeneralCollapsible/ChevronToggle

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs the text so not as is, but I extended the Chevron to accept text and size inputs to make it work. Might be useful down the line in other places

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if you use data-collapsible-click-region=true?

Comment thread lua/wikis/commons/PrizePool/Base.lua Outdated
Comment on lines +347 to +348
return TableCell{children = {table.concat(headerDisplay)}}
return TableCellHeader{children = {table.concat(headerDisplay)}, align = 'right'}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

table.concat is redundant

Comment on lines +724 to +727
---@param placement BasePlacement
---@param opponent table
---@return Renderable[]
function BasePrizePool:_opponentPrizeCells(placement, opponent)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
---@param placement BasePlacement
---@param opponent table
---@return Renderable[]
function BasePrizePool:_opponentPrizeCells(placement, opponent)
---@private
---@param placement BasePlacement
---@param opponent BasePlacementOpponent
---@return Renderable[]
function BasePrizePool:_opponentPrizeCells(placement, opponent)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract column definitions to separate field in prize types table instead of adding all the aligns in cells
same for opponent column too

@ElectricalBoy

Copy link
Copy Markdown
Collaborator

image export config for prize pools should be updated as well

@Eetwalt Eetwalt requested a review from ElectricalBoy June 25, 2026 09:21
@Eetwalt

Eetwalt commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

since we are at it this should also take room for currency switch into account

This can be in a separate PR imo

@hjpalpha

Copy link
Copy Markdown
Collaborator

since we are at it this should also take room for currency switch into account

This can be in a separate PR imo

there already is #7687
but it will obviously get a shit ton of merge conflicts with this PR :P

@ElectricalBoy

Copy link
Copy Markdown
Collaborator

there already is #7687 but it will obviously get a shit ton of merge conflicts with this PR :P

there's no way that there won't be a merge conflict
that thing is grid-based :p

Comment on lines +439 to +478
&.prizepooltable-placement {
tr.table2__row--body.background-color-first-place {
border-bottom: 0;

&,
&:hover {
background-color: var( --prizepool-place-1-tint );
}

.prizepooltable-place::before {
background-color: var( --prizepool-place-1-accent );
}
}

tr.table2__row--body.background-color-second-place {
border-bottom: 0;

&,
&:hover {
background-color: var( --prizepool-place-2-tint );
}

.prizepooltable-place::before {
background-color: var( --prizepool-place-2-accent );
}
}

tr.table2__row--body.background-color-third-place {
border-bottom: 0;

&,
&:hover {
background-color: var( --prizepool-place-3-tint );
}

.prizepooltable-place::before {
background-color: var( --prizepool-place-3-accent );
}
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to have a separate color for hovering to keep consistency with other rows (like how standings table handles hovering)

&:hover {
> :nth-child( -n + 2 ),
&:has( > :nth-child( 2 ) > .standings-position-indicator ) > :nth-child( 3 ) {
background-color: hsl( from var( --placement-confirmed-color ) h s calc( l * 0.9 ) );
.theme--dark & {
background-color: hsl( from var( --placement-confirmed-color ) h s calc( l * 1.25 ) );
}
}
}
}

Comment on lines +403 to +433
&.prizepooltable-placement .prizepooltable-place {
text-align: center;
font-weight: bold;
color: var( --clr-secondary-25 );

// Override the legacy `… > td.prizepooltable-place` metal fill so the row
// tint + accent bar show through on the redesigned table instead.
background-color: transparent;

.theme--dark & {
color: var( --clr-secondary-100 );
}
}

// Top-3 rows: left accent bar (coloured, with the row tint, in the theme block below).
&.prizepooltable-placement {
tr.table2__row--body.background-color-first-place,
tr.table2__row--body.background-color-second-place,
tr.table2__row--body.background-color-third-place {
.prizepooltable-place {
position: relative;

&::before {
content: "";
position: absolute;
inset: 0 auto 0 0;
width: $prizepool-accent-width;
}
}
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
&.prizepooltable-placement .prizepooltable-place {
text-align: center;
font-weight: bold;
color: var( --clr-secondary-25 );
// Override the legacy `… > td.prizepooltable-place` metal fill so the row
// tint + accent bar show through on the redesigned table instead.
background-color: transparent;
.theme--dark & {
color: var( --clr-secondary-100 );
}
}
// Top-3 rows: left accent bar (coloured, with the row tint, in the theme block below).
&.prizepooltable-placement {
tr.table2__row--body.background-color-first-place,
tr.table2__row--body.background-color-second-place,
tr.table2__row--body.background-color-third-place {
.prizepooltable-place {
position: relative;
&::before {
content: "";
position: absolute;
inset: 0 auto 0 0;
width: $prizepool-accent-width;
}
}
}
}
&.prizepooltable-placement {
.prizepooltable-place {
text-align: center;
font-weight: bold;
color: var( --clr-secondary-25 );
// Override the legacy `… > td.prizepooltable-place` metal fill so the row
// tint + accent bar show through on the redesigned table instead.
background-color: transparent;
.theme--dark & {
color: var( --clr-secondary-100 );
}
}
// Top-3 rows: left accent bar (coloured, with the row tint, in the theme block below).
tr.table2__row--body.background-color-first-place,
tr.table2__row--body.background-color-second-place,
tr.table2__row--body.background-color-third-place {
.prizepooltable-place {
position: relative;
&::before {
content: "";
position: absolute;
inset: 0 auto 0 0;
width: $prizepool-accent-width;
}
}
}
}

Comment on lines +42 to +43
props = props or {}
local size = props.size or 'xs'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use defaultProps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: prize_pool c: standard javascript Changes to JavaScript files stylesheets Changes to stylesheets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants