Skip to content

Conversation

@jasonyess
Copy link
Contributor

An implementation of #208, adds a dropdown which allows the user to choose between two sorting options

image

The selected value is saved in cookies, where the default value of the simple_dropdown is set to the cookie value.

In the Javascript (only in the IndividualStatsViewer class), the code that populates the stats containers is moved from the onReceive function to its own populateStatsContainers function, so the stats containers can be repopulated without sending another request

Currently, it's only implemented in the individual stats viewer, but I could see this feature being nice to have in the nation stats viewer as well so I'm open to making changes

I'm not entirely sure if handling all of the cookie stuff in the backend is the greatest idea, my second thought would be to just modify the dropdown's selected value to the cookie value (or maybe just a value in localStorage) once it's initialized in the JS, but I'm not quite sure

License Acceptance

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Jan 11, 2025

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 30.34%. Comparing base (d1269c2) to head (6f5dec4).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...ointercrate-demonlist-pages/src/statsviewer/mod.rs 0.00% 16 Missing ⚠️
...rate-demonlist-pages/src/statsviewer/individual.rs 0.00% 1 Missing ⚠️
...rcrate-demonlist-pages/src/statsviewer/national.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
- Coverage   30.41%   30.34%   -0.07%     
==========================================
  Files         114      114              
  Lines        8019     8037      +18     
==========================================
  Hits         2439     2439              
- Misses       5580     5598      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stadust
Copy link
Owner

stadust commented Jan 11, 2025

I'm not entirely sure if handling all of the cookie stuff in the backend is the greatest idea, my second thought would be to just modify the dropdown's selected value to the cookie value (or maybe just a value in localStorage) once it's initialized in the JS, but I'm not quite sure

Mhh, yeah, sorry, suggesting cookies was wrong of me, I meant local storage 😭 I meant localstorage. The tabbed pane stuff (e.g. what allows you to select tabs in the user area and restores your selects tab after reloading) in pointercrate-core-pages/static/js/modules/tab.js already does something like this.

@stadust
Copy link
Owner

stadust commented Jan 11, 2025

Currently, it's only implemented in the individual stats viewer, but I could see this feature being nice to have in the nation stats viewer as well so I'm open to making changes

Let's do it for both, the initialization of the dropdown can be moved into the stats viewer base class, and from there it can probably even re-trigger the abstract population routine.

I also still think that when its set to "by difficulty", it shouldn't populate everything into the same list, but rather use separate lists for main/extended/legacy demons.

fwiw,

@jasonyess jasonyess changed the title Add dropdown to sort completed demons on individual stats viewer Add dropdown to sort completed demons on stats viewers Jan 11, 2025
@jasonyess
Copy link
Contributor Author

When sorting by difficulty, I implemented the separate lists you talked about
I haven't implemented them to the nation stats viewer yet, but please tell me what I can change

image
image

("None" is replaced with a dash just because it was big compared to the section titles)

In the rust, I also modified the StatsViewerRow struct so it's really easy to have multiple sections in one StatsViewerRow
image

@stadust
Copy link
Owner

stadust commented Jan 11, 2025

I'll need to have an indepth look at the code tomorrow (looks pretty good at first glance though!), but UI wise I was more thinking of something like this:
image

@jasonyess
Copy link
Contributor Author

oh 😭 i guess i didn't need to modify StatsViewerRow, i might as well leave it though since maybe it'll be useful at some point

@stadust stadust force-pushed the master branch 3 times, most recently from 62df64c to be0d5b7 Compare March 8, 2025 14:45
@stadust stadust force-pushed the stats-viewer-demon-sorting-options branch 2 times, most recently from 954c66f to 84535d7 Compare March 14, 2025 21:06
…sition

Cookie fix

Rename to_cookie to to_value

Move from use of cookies to localStorage

Add sorting options to nation stats viewer

Added ability to have multiple sections within one StatsViewerRow

List demons differently when sorting by position in individual stats viewer

Replaced unnecessary use of formatDemon

Modified UI

Fix stats containers not being hidden properly

Implemented sectioned lists to nation stats viewer

from stadust:

* eliminate `Vec` from `StatsViewerRow` again

* dont use jQuery for hiding/showing html elements

* ui: pre-create both alphabetical and position sorted demons

  Always populate eagerly both the stats viewer divs for alphebatical
  demon sorting and position sorting, so that the dropdown selection only
  needs to show/hide some divs instead of creating a bunch of UI.
@stadust stadust force-pushed the stats-viewer-demon-sorting-options branch from 84535d7 to 6f5dec4 Compare March 14, 2025 21:14
@stadust stadust merged commit 166e3b2 into stadust:master Mar 14, 2025
2 of 4 checks passed
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.

2 participants