Skip to content

refactor: overhaul notability checker display - #8089

Draft
ElectricalBoy wants to merge 9 commits into
mainfrom
notability-checker-widget3
Draft

ElectricalBoy wants to merge 9 commits into
mainfrom
notability-checker-widget3

Conversation

@ElectricalBoy

Copy link
Copy Markdown
Collaborator

Summary

This PR:

  • replaces template expansions in notability checker with direct lua calls
  • replaces raw HTML strings with widget3 HTML components

How did you test this change?

untested

@hjpalpha hjpalpha 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.

fwiw i still think that the entire notability checker setup could eventually use a clean rewrite that actually supports all wikis

Comment thread lua/wikis/commons/NotabilityChecker.lua Outdated
for _, person in pairs(people) do
local personWeight = NotabilityChecker._calculatePersonNotability(person)
output = output .. mw.getCurrentFrame():expandTemplate{
title = 'NotabilityPlayerMatchesTable', args = {title = person}}

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.

fwiw this might exist on some wikis with overwriting the normal results table

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.

I would argue that the local overrides should be added to /Custom
plus imo having this unnecessary template expansion is just more evil

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.

tend to agree
but you will likely get pushback from some of the affected wikis

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 know at least some wikis have versions of it that displays the score per result, which i doubt they want to lose. Could add that to ResultsTable/Base i guess.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants