Skip to content

refactor: make mergeProps more efficient (at cost of mutating)#7487

Merged
Rathoz merged 2 commits into
mainfrom
simplify-table2-mergeprops
May 11, 2026
Merged

refactor: make mergeProps more efficient (at cost of mutating)#7487
Rathoz merged 2 commits into
mainfrom
simplify-table2-mergeprops

Conversation

@Rathoz
Copy link
Copy Markdown
Collaborator

@Rathoz Rathoz commented May 11, 2026

Summary

More efficient, but mutates. However many of the other functions in the same util also mutates the inputs.

Also fixes https://discord.com/channels/93055209017729024/372075546231832576/1503394166829224046

How did you test this change?

dev

@Rathoz Rathoz requested review from a team as code owners May 11, 2026 15:25
Copilot AI review requested due to automatic review settings May 11, 2026 15:25
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 refactors ColumnUtil.mergeProps in the Table2 widget to avoid creating a separate merged props table, instead mutating the passed-in cell/header props for improved efficiency.

Changes:

  • Refactored ColumnUtil.mergeProps to mutate cellProps and introduced a shared INHERITABLE_PROPS list.
  • Updated Table2 Cell and CellHeader to rely on mutated props instead of a returned merged table.
  • Simplified the call sites by removing the intermediate mergedProps variable.

Reviewed changes

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

File Description
lua/wikis/commons/Widget/Table2/ColumnUtil.lua Refactors prop merging to mutate input props and centralizes inheritable prop keys.
lua/wikis/commons/Widget/Table2/CellHeader.lua Adjusts header rendering to use mutated props after merging.
lua/wikis/commons/Widget/Table2/Cell.lua Adjusts cell rendering to use mutated props after merging.

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

Comment thread lua/wikis/commons/Widget/Table2/ColumnUtil.lua
Comment thread lua/wikis/commons/Widget/Table2/ColumnUtil.lua Outdated
Copy link
Copy Markdown
Collaborator

@hjpalpha hjpalpha left a comment

Choose a reason for hiding this comment

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

lgtm on phone

@Rathoz Rathoz merged commit 2f8085c into main May 11, 2026
9 checks passed
@Rathoz Rathoz deleted the simplify-table2-mergeprops branch May 11, 2026 17:22
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