Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebHost: Filter the multitracker summary row using applied search filter #3257

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

remyjette
Copy link
Collaborator

What is this fixing or adding?

The summary row added in #1965 always shows the status of the entire multiworld, regardless of any filter settings applied. This post in #general-suggestions https://discord.com/channels/731205301247803413/1231050777121001482 suggested that if a search filter is applied, the search filter should also apply to the summary. That way it would be easy for a player to easily see the percentage of their own checks they've completed across the multiworld. I agree with that suggestion, so here's an implementation.

Note that with this change the multiTracker.html template doesn't really need to populate the footer at all. And thus we can probably remove a view variables from render_generic_multiworld_tracker's call to render_template(). I did not make that part of this PR but am happy to do so if that is desired.

How was this tested?

I was a bit concerned about performance since this is now calculating it from the data in the table, so I also tested with a 2000-player multi-world (thank you Factorio for being fast to generate). The footerCallback took 4ms-6ms to run with the whole table. After searching for Player100 which gave me the 12 playes Player100 and Player1000-1009 it took 0.4ms.

I also tested it by just playing around with the search on a copy of the db of a multiworld on my webhost I have in progress, to make sure everything looked right. Also made sure to test empty result as well as ensure things kept working after update was called.

If this makes graphical changes, please attach screenshots.

image

image

image

image

@github-actions github-actions bot added affects: webhost Issues/PRs that touch webhost and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels May 3, 2024
@remyjette remyjette added the is: enhancement Issues requesting new features or pull requests implementing new features. label May 3, 2024
@LegendaryLinux
Copy link
Member

LegendaryLinux commented May 16, 2024

I suggest you add a checkbox to toggle this functionality. If the box is checked, the summary row shows only the summary of displayed rows, and if the box is not checked, it shows the summary of all rows. The checkbox should be above the table and below the search bar, and be labelled "Summarize Visible Rows." Default not checked.

Copy link
Member

@LegendaryLinux LegendaryLinux left a comment

Choose a reason for hiding this comment

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

See above comment.

@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: author Issue/PR is waiting for feedback or changes from its author. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants