search

magnusbakken / espn-fantasy-autopick

A Chrome extension that lets you automatically add active players to the current roster in an ESPN NBA/NHL fantasy league.
MIT License
11 stars 6 forks source link

[NHL] Additional goalie fixes #14

Closed cameronkeif closed 3 years ago

cameronkeif commented 3 years ago

There are two edge cases around goalies that were missed in #11 around the goalie tables.

  1. The code was trying to find the "Move" button in the IR slot if a player was in there. I did some playing around and it doesn't seem that the code moves players to/from IR anyway (there is a separate "Manage IR" button in ESPN to handle this), so I just removed those slots from the rows calculation as they don't seem to be needed. If I am missing a case where they are needed, please let me know and I can try to find an alternative solution.

  2. Categories leagues were not working, because a points league adds two additional tables, so getRosterRows resulted in an error in category leagues, due the position slot being off by one. I updated this to explicitly test the length and find playerTable2 accordingly.

Demo (both a category and points league) nhl-fix2

cameronkeif commented 3 years ago

"edge" cases is a bit of a misnomer, more like "normal cases that @cameronkeif just didn't test tsk tsk" :)

magnusbakken commented 3 years ago

Concerning the IR issue, I'm not sure why I decided at some point to put IR players in the bench list. Maybe it was just for the sake of completeness, but as you say the extension will never touch them so it's probably easier to just pretend they don't exist.

There's some other code that should've been excluding players in IR slots, including the isInjuredReserve field on the Player class and the injuredReserves list on the RosterState class. I don't know if I understand why this part works differently between the different sports (maybe something to do with the multiple tables in the HTML), but I think it should be fine to exclude IR players but keep the other code around for now.

About the multiple tables, could there be something to base the query for the tables on that isn't .players-table tbody.Table__TBODY selector, where we would only get the 1 or 2 tables we care about? The tableBody.length / 2 thing is a little confusing, but I'm okay with it if there's nothing better to distinguish them.

cameronkeif commented 3 years ago

Concerning the IR issue, I'm not sure why I decided at some point to put IR players in the bench list. Maybe it was just for the sake of completeness, but as you say the extension will never touch them so it's probably easier to just pretend they don't exist.

Yeah, that's kinda what I was thinking too. I didn't see any code path that would do anything with them.

There's some other code that should've been excluding players in IR slots, including the isInjuredReserve field on the Player class and the injuredReserves list on the RosterState class. I don't know if I understand why this part works differently between the different sports (maybe something to do with the multiple tables in the HTML), but I think it should be fine to exclude IR players but keep the other code around for now.

I wonder if maybe it wasn't impacting the one-table sports because those rows were at the very end, so you wouldn't be moving a player up into them? Not really sure :sweat_smile: .

About the multiple tables, could there be something to base the query for the tables on that isn't .players-table tbody.Table__TBODY selector, where we would only get the 1 or 2 tables we care about? The tableBody.length / 2 thing is a little confusing, but I'm okay with it if there's nothing better to distinguish them.

Yeah, I see what you're saying, we could probably doll it up with a better selector. I was thinking that an indexed-based approach would be safer than relying on ESPN's CSS class names that seem to constantly change, but both cases are probably equally likely to change so it's not possible to be completely defensive anyway.

If you'd prefer, we could change the selector to '.players-table table.Table--fixed-left tbody.Table__TBODY'. The table containing the players is the only one that includes the class Table--fixed-left. ' .players-table__sortable > .players-table:first-of-type > div > table:first-of-type' could also work as an alternative (ESPN seems to just pepper these classes everywhere so it's hard to make a clean selector lol). I've attached a .gif showcasing this.

Let me know what you think and I can change if you prefer that approach! Or if you have any other ideas for a better selector I can easily change it.

dom

magnusbakken commented 3 years ago

I like the fixed-left option. Even if they change the class names, they probably won't change the fact that the players are on the left side of the screen.

cameronkeif commented 3 years ago

Cool, I agree, it seems like the best combination between being readable and being defensive of the 3 options.

I updated and retested in both leagues shown in the above .gif in 90608a79b2bff935336b72c9d49cf22bc3fca74a.

magnusbakken commented 3 years ago

And I've confirmed that your latest version still works well for my NBA league (and my weird test league with strange settings), so I'll merge this. Thanks again for the great work!

cameronkeif commented 3 years ago

Awesome, thanks @magnusbakken ! Let me know when you publish it to the store and I'll re-test the live version.

magnusbakken commented 3 years ago

I've submitted the new version to the store. I didn't set it to auto-publish so it probably won't be up until tomorrow, unless the review comes through within a couple of hours.