Skip to content

refactor: basic widgets to widget3#7482

Open
ElectricalBoy wants to merge 35 commits into
mainfrom
basic-widgets-to-widget3
Open

refactor: basic widgets to widget3#7482
ElectricalBoy wants to merge 35 commits into
mainfrom
basic-widgets-to-widget3

Conversation

@ElectricalBoy
Copy link
Copy Markdown
Collaborator

How did you test this change?

preview with dev

@ElectricalBoy ElectricalBoy requested review from a team as code owners May 11, 2026 03:39
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.

technically that makes lots of annos wrong in the consumer modules ;)
and needs test cases fixed
apart from that lgtm

Comment thread lua/wikis/commons/Widget/Basic/Dropdown/Container.lua Outdated
Comment thread lua/wikis/commons/Widget/Basic/Dropdown/Container.lua Outdated
@ElectricalBoy ElectricalBoy requested a review from Rathoz May 11, 2026 07:42
Comment thread lua/wikis/commons/Widget/Basic/Dropdown/Container.lua Outdated
Comment thread lua/wikis/commons/Widget/Basic/Dropdown/Container.lua Outdated
@ElectricalBoy ElectricalBoy requested review from Rathoz and hjpalpha May 11, 2026 09:55
@ElectricalBoy ElectricalBoy force-pushed the basic-widgets-to-widget3 branch 2 times, most recently from c58d5ad to 32bb3c4 Compare May 11, 2026 12:02
@ElectricalBoy ElectricalBoy force-pushed the basic-widgets-to-widget3 branch from 062892a to 2937b0e Compare May 11, 2026 12:13
Comment on lines +17 to +21
---@class HtmlNodeProps
---@field classes? string[]
---@field css? table<string, string|number?>
---@field attributes? table<string, string|number?>
---@field children? Renderable|Renderable[]
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.

Let's use a multi-line alias instead, so we are consistent between alias and class (as they behave differently)

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.

---@class ButtonWidgetParameters: HtmlNodeProps

---@class DataTableProps: HtmlNodeProps

there are few places like the above two that simply extend HtmlNodeProps to save some duplicate code

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.

This one can't be done until defaultProps fromTemplate combo is fixed. Can you check if any others needs that too?

Copy link
Copy Markdown
Collaborator Author

@ElectricalBoy ElectricalBoy May 13, 2026

Choose a reason for hiding this comment

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

there are 7 widgets in this PR that use defaultProps

  • dropdown/container
  • dropdown/item
  • button
  • carousel
  • datatable
  • link
  • iconimage

afaik button is the only one using widget/factory entrypoint; we could either get #7478 deployed first or defer button migration. Adding temporary workaround like #7477 is an option too

@ElectricalBoy ElectricalBoy requested a review from Rathoz May 13, 2026 08:44
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