Skip to content

Conversation

@ElectricalBoy
Copy link
Collaborator

@ElectricalBoy ElectricalBoy commented Jun 27, 2025

Summary

#6076 added stats for heralds, void grubs and atakhan to LoL match pages. However, for older matches that were played before some of these objectives were introduced, it only introduces redundant rows that do nothing.

This PR adds match date-based conditions to suppress display of objectives that were not present at the time of match.

How did you test this change?

dev: https://liquipedia.net/leagueoflegends/Match:ID_User_ElectricalBoy_zqx7pe0nDj_R01-M001

Copy link
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.

seems reasonable on phone

@Rathoz
Copy link
Collaborator

Rathoz commented Jun 28, 2025

Nah, doesn't belong in the frontend. If it should be added, it belongs in the backend

@ElectricalBoy ElectricalBoy requested a review from hjpalpha June 29, 2025 01:04
Copy link
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.

codewose okay (on phone)

@Rathoz
Copy link
Collaborator

Rathoz commented Jul 3, 2025

I think this belongs to the proper proper backend (ie loldb), we would need to fix it there for the future anyway

@ElectricalBoy
Copy link
Collaborator Author

ElectricalBoy commented Jul 3, 2025

I think this belongs to the proper proper backend (ie loldb), we would need to fix it there for the future anyway

I agree, but that would have to wait until Crossroads migration, right?
I think this patch suffices as a temporary solution.

@ElectricalBoy ElectricalBoy requested a review from Rathoz July 18, 2025 08:18
@ElectricalBoy ElectricalBoy requested a review from Rathoz September 8, 2025 01:12
@ElectricalBoy ElectricalBoy force-pushed the lol-matchpage-new-stats-in-new-games-only branch from d4b09b6 to eace774 Compare September 8, 2025 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants