downforacross / downforacross.com

Web frontend for downforacross.com -- continuation of stevenhao/crosswordsio
https://downforacrosscom.downforacross1.now.sh
MIT License
220 stars 92 forks source link

Show number of squares filled by each user in Color Attribution Mode #287

Closed mitchellvitez closed 9 months ago

mitchellvitez commented 10 months ago

Inspired by https://github.com/downforacross/downforacross.com/issues/286

Shows the number of squares filled in by each player at each point on the Replay timeline

When scrubbed to the end of the Replay timeline, this will be the number of squares solved by each player

downforacross squarecounts

downforacross squarecounts

vercel[bot] commented 10 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
downforacross.com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 27, 2023 9:28pm
stevenhao commented 10 months ago

Thanks for this PR! I'm not sure whether this feature would be redundant with the color attribution mode. And I'm also not sure if it needs to be always-showing, given that only a subset of users would find it helpful.

Perhaps we could make this show up only if color attribution mode is enabled? And render the usernames with colors as well?

mitchellvitez commented 10 months ago

Agreed that only a subset of users would find this helpful/interesting

For us, we often have 5–10 people playing so the colors can be a little hard to distinguish. I'd also argue it's not totally redundant because it saves us from having to count up squares manually to find this user ordering.

I think showing up only in color attribution mode (with user colors) is a great idea, just made that change:

Screenshot 2023-09-03 at 10 17 41 AM
ekilah commented 10 months ago

Excited for this!

jsphwllng commented 9 months ago

This is exactly what I hoped for with my issue. Thanks for this :sparkle:

stevenhao commented 9 months ago

Sorry for being slow to review this! I think if we fix the sort thingy we can get it merged. Thanks for your patience :)

mitchellvitez commented 9 months ago

No worries about timing

I took a stab at making the sorting more clear. If that's not what you meant, can you say more about what you want fixed with the sort?

I checked that

typeof(Object.entries(counts)[0][1])

was number in the original

stevenhao commented 9 months ago

No worries about timing

I took a stab at making the sorting more clear. If that's not what you meant, can you say more about what you want fixed with the sort?

I checked that

typeof(Object.entries(counts)[0][1])

was number in the original

I think the way you're invoking the function might not be right. The docs here should be helpful https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort

mitchellvitez commented 9 months ago

Ah you're right, great catch! I had assumed sort would take a property to sort on, not a comparison function

Should be fixed now

stevenhao commented 9 months ago

awesome work!!