Skip to content

Conversation

@sphericle
Copy link
Contributor

closes #222

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.

@sphericle
Copy link
Contributor Author

ok now how do I select something from the list programmatically 😭

@sphericle sphericle changed the title accept parameter, pass it through to class init Link to statsviewer player Apr 4, 2025
@sphericle sphericle marked this pull request as draft April 4, 2025 01:14
@codecov
Copy link

codecov bot commented Apr 4, 2025

Codecov Report

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

Project coverage is 32.16%. Comparing base (cf7477b) to head (6dd8e25).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...te-demonlist-pages/src/account/list_integration.rs 0.00% 1 Missing ⚠️
pointercrate-demonlist-pages/src/demon_page.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #241      +/-   ##
==========================================
- Coverage   32.16%   32.16%   -0.01%     
==========================================
  Files         118      118              
  Lines        8132     8133       +1     
==========================================
  Hits         2616     2616              
- Misses       5516     5517       +1     

☔ 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 Apr 4, 2025

probably easier to do this completely on the frontend - e.g. have the onLoad handler parse the URI, see if a player is specified, and then just select it with statsViewer.selectArbitrary()

@sphericle
Copy link
Contributor Author

that sounds like a way better idea lol. will look at it at some point tonight

@sphericle
Copy link
Contributor Author

Made those changes, but now to actually select the user with selectArbitrary(playerId), statsViewer.currentlySelected has to be defined :/

@sphericle sphericle closed this Apr 5, 2025
@sphericle sphericle reopened this Apr 5, 2025
@sphericle
Copy link
Contributor Author

LOL I pressed the wrong button

@sphericle
Copy link
Contributor Author

sphericle commented Apr 6, 2025

ok i'm 90% sure selectArbitrary(playerId) won't work because the score and rank rely on currentlySelected. i tried finding the player's li element from statsViewer.list and using onSelect(li) but this would only work if the linked player is top 50 (on the first page of the paginator).

will do some more digging and maybe find a cool method that solves this later (🙏🙏🙏🙏). alternatively i can just change the score and rank to not use currentlySelected idk maybe we'll see

@sphericle sphericle marked this pull request as ready for review April 7, 2025 20:21
@sphericle
Copy link
Contributor Author

Here I made a new endpoint that returns a RankedPlayer object for the given id. I picked RankedPlayer because I think its the lightest player struct with a rank, but if you want me to change this to FullPlayer or something let me know :)

@sphericle
Copy link
Contributor Author

sphericle commented Apr 7, 2025

opened a PR for documentation aswell
stadust/pointercrate-documentation#4

@sphericle
Copy link
Contributor Author

this is pretty much done! let me know if you need me to change anything :D

@stadust
Copy link
Owner

stadust commented May 25, 2025

Hey @sphericle, I've gone ahead and implemented the whole "include rank in FullPlayer objects" thingy, and fixed up the stats viewer frontend to no longer rely on parsing the HTML from the player list on the left, so selectArbitrary() should actually work now for your purposes. Could you rebase your PR on top of master, to see if that's enough to get the deep link support working? :o

@sphericle sphericle force-pushed the statsviewer-links branch from 85148ad to 0a33299 Compare May 27, 2025 20:00
@sphericle
Copy link
Contributor Author

Can confirm that it does work now, thanks :)
I also adjusted the "Go to statsviewer" button as well as (re)adding some error handling and handling of banned users

Copy link
Owner

@stadust stadust left a comment

Choose a reason for hiding this comment

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

Thanks!

Now that we have the deep links in place, maybe we can go a bit further still, and turn all the player names on the record lists on the demon pages into stats viewer deep links? :o

sphericle and others added 2 commits May 28, 2025 16:16
@sphericle
Copy link
Contributor Author

sphericle commented May 28, 2025

Added the deep links to the record list. I'm personally a bit iffy on putting the link on the record's username, as I think most people are used to the completion link being there, but ig it's pretty obvious that you can just click "youtube" and go to the video anyways

@stadust
Copy link
Owner

stadust commented May 28, 2025

Added the deep links to the record list. I'm personally a bit iffy on putting the link on the record's username, as I think most people are used to the completion link being there, but ig it's pretty obvious that you can just click "youtube" and go to the video anyways

Ah, I completely forgot it did that. And it needs to continue doing that because on mobile, the "youtube" column doesn't exist, ugh. Maybe we can just move the youtube link to the percentage in that case (and actually style it like a link)? People will grumble, and it'll take some time getting used to, but I really wanna start sprinkling these stats viewer deep links in there haha.

@jasonyess
Copy link
Contributor

how about we just have a link icon (without the text) in the progress column next to the percentage thats only revealed on mobile? then the player name could link to the player, and the video column can still exist

something like this:
image

not really my lane i just thought it was a neat suggestion lol

@stadust
Copy link
Owner

stadust commented May 29, 2025 via email

@sphericle
Copy link
Contributor Author

sphericle commented May 30, 2025

new.mp4

I added the styling to the whole percentage text because it was easier lol, but i can take a closer look at only putting it on the little box icon if needed

sphericle and others added 5 commits May 29, 2025 22:20
zero is false-y, so it was triggering the path for "malformed player
id". Does not really make a difference because there will never be a
player with id 0, but oh well.

Signed-off-by: stadust <43299462+stadust@users.noreply.github.com>
Otherwise it'll just stick around forever, which is a bit annoying if
someone clicks a stats viewer link for a non-existing player.

Signed-off-by: stadust <43299462+stadust@users.noreply.github.com>
- remove copy-to-clipboard when clicking player name. The on-hover
  underlining was causing the flags to resize, and now that the URL gets
updated, one can just copy the link from the browser (which is a bit
more intuitive imo, as nothing was really indicating that the clipboard
functionality was there, or that "add to clipboard" was actually what
happened when the name was clicked)
- Gave small visual indication that player names on demon pages can be
  clicked by underlining them.

Signed-off-by: stadust <43299462+stadust@users.noreply.github.com>
By using direct stats viewer links, its possible to pull up players that
have no score, and hence no rank. Display rank as '-' for these.

Signed-off-by: stadust <43299462+stadust@users.noreply.github.com>
@stadust stadust enabled auto-merge (squash) May 30, 2025 16:50
@stadust stadust merged commit ff1283a into stadust:master May 30, 2025
2 checks passed
@sphericle sphericle deleted the statsviewer-links branch December 22, 2025 01:34
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.

Stats viewer profile links

3 participants