Skip to content

refactor: migrate table2 to widget3#7428

Open
Rathoz wants to merge 9 commits intomainfrom
widget3-table2
Open

refactor: migrate table2 to widget3#7428
Rathoz wants to merge 9 commits intomainfrom
widget3-table2

Conversation

@Rathoz
Copy link
Copy Markdown
Collaborator

@Rathoz Rathoz commented Apr 22, 2026

Summary

Migrate table2 to use widget3. This may temporarily break some Squad functionality. But Squad is stacked on this one, so a quick merge between the two.

How did you test this change?

dev with squad, but some independent table2 tests would be useful

@Rathoz Rathoz changed the title refactor: create a component version of table2 refactor: create a component (widget3) version of table2 Apr 28, 2026
@Rathoz Rathoz force-pushed the widget3-table2 branch from 9201659 to 697648d Compare May 4, 2026 14:49
@Rathoz Rathoz changed the title refactor: create a component (widget3) version of table2 refactor: migrate table2 to widget3 May 5, 2026
@Rathoz Rathoz marked this pull request as ready for review May 5, 2026 08:14
@Rathoz Rathoz requested review from a team as code owners May 5, 2026 08:14
Comment thread lua/wikis/commons/Widget/Table2/Cell.lua
Base automatically changed from widget30 to main May 7, 2026 08:56
Copilot AI review requested due to automatic review settings May 7, 2026 08:59
@Rathoz Rathoz force-pushed the widget3-table2 branch from 7cc64fc to 57af813 Compare May 7, 2026 08:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 migrates the Table2 widget set from the legacy Widget (class-based) system to widget3 functional components + ComponentContext, aligning Table2 with the newer rendering pipeline.

Changes:

  • Refactors Table2 components (Table, Header, Body, Row, Cell, CellHeader) to Component.component(...) and widget3 Html.
  • Replaces legacy Widget contexts with ComponentContext providers/readers.
  • Updates Table2 context definitions to use Context.create(...).

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
lua/wikis/commons/Widget/Table2/TableHeader.lua Migrates header to widget3 component + context provider usage.
lua/wikis/commons/Widget/Table2/TableBody.lua Migrates body to widget3 component and rewrites striping/rowspan logic.
lua/wikis/commons/Widget/Table2/Table.lua Migrates table wrapper to widget3 + new Html module; reintroduces defaults via Component.component.
lua/wikis/commons/Widget/Table2/Row.lua Migrates row rendering to widget3 and updates context reads + cell indexing logic.
lua/wikis/commons/Widget/Table2/CellHeader.lua Migrates header cell to widget3 and updates context reads + Html usage.
lua/wikis/commons/Widget/Table2/Cell.lua Migrates cell to widget3 and updates context reads + Html usage.
lua/wikis/commons/Widget/Table2/All.lua Renames internal export table variable; continues exporting Table2 entrypoints.
lua/wikis/commons/Widget/Contexts/Table2.lua Replaces Widget2 contexts with widget3 ComponentContext definitions.
Comments suppressed due to low confidence (1)

lua/wikis/commons/Widget/Table2/Row.lua:86

  • Cell/header detection uses getmetatable(child) comparisons against component metatables. For widget3 components, all VNodes share the same metatable, so this will never match and columnIndex/colspan inference won’t run. Use a VNode identity check (e.g., type(child)=='table' and child.renderFn == Table2Cell.renderFn / Table2CellHeader.renderFn) instead.
	local indexedChildren = Array.map(children, function(child)
		if getmetatable(child) == getmetatable(Table2Cell) or getmetatable(child) == Table2CellHeaderMT then
			local cellChild = child
			local explicitIndex = MathUtil.toInteger(cellChild.props.columnIndex)
			if explicitIndex and explicitIndex >= 1 then
				columnIndex = math.max(columnIndex, explicitIndex)
			elseif cellChild.props.columnIndex == nil then
				cellChild.props.columnIndex = columnIndex
			end

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

Comment thread lua/wikis/commons/Widget/Table2/TableHeader.lua
Comment thread lua/wikis/commons/Widget/Table2/TableBody.lua
Comment thread lua/wikis/commons/Widget/Table2/TableBody.lua
Comment thread lua/wikis/commons/Widget/Table2/TableBody.lua
Comment thread lua/wikis/commons/Widget/Contexts/Table2.lua Outdated
Comment thread lua/wikis/commons/Widget/Table2/Row.lua
Comment thread lua/wikis/commons/Widget/Table2/Table.lua
Comment thread lua/wikis/commons/Widget/Table2/TableHeader.lua
Comment thread lua/wikis/commons/Widget/Table2/TableBody.lua Outdated
Comment thread lua/wikis/commons/Widget/Table2/Row.lua

---@class Table2CellProps
---@field children? Renderable|Renderable[]
---@field children Renderable[]
Copy link
Copy Markdown
Collaborator

@hjpalpha hjpalpha May 7, 2026

Choose a reason for hiding this comment

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

we use it with non arrays quite often currently

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants