Use avatars for teams without manually-submitted logos #140
Conversation
| @@ -260,13 +269,13 @@ function openInfo(marker) { | |||
| } else { | |||
| content += '<h1>' + parsed.short_name + '</h1>' | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Woah, thanks for removing the trailing spaces. Not sure how that got in there.
| var custom = icons.indexOf(title) !== -1 | ||
| var imageUrl = 'resources/marker.png' | ||
| if (custom){ | ||
| imageUrl = 'logos/' + title + '.png' |
There was a problem hiding this comment.
Hmm, now that you've programmed this I'm actually inclined to phase out manual logo specification. It makes more sense to just use the team's avatar from TBA.
Do you think you could make the TBA avatar take precedence?
There was a problem hiding this comment.
Actually let's keep giving precedence to our logos. Looking at 4188's logo, it wouldn't be so great to use the one they have on TBA.
There was a problem hiding this comment.
We also don't have any assurance that avatars will continue to be a thing after Destination Deep Space season opens; they may just be a Power Up thing.
There was a problem hiding this comment.
Going back to this, they did continue—I think this is worth another look #197
| var custom = icons.indexOf(title) !== -1 | ||
| var imageUrl = 'resources/marker.png' | ||
| if (custom){ |
There was a problem hiding this comment.
Fix the spacing in this section. The rest of the document is using four-space tabs. Also put a space before the {.
| var imageUrl = 'resources/marker.png' | ||
| if (custom){ | ||
| imageUrl = 'logos/' + title + '.png' | ||
| } else { |
| } else { | ||
| if (teamAvatars[title]) { | ||
| custom=true; | ||
| imageUrl = 'data:image/png;base64,' + teamAvatars[title]["img"] |
There was a problem hiding this comment.
I'm not a huge fan of duplicating a bunch of Base64 image data. Can you figure out a way to pull the images from TBA automatically? If not, that's fine, but it would be nicer that way.
There was a problem hiding this comment.
Also, use single quotes (') to match the rest of the file and space out your equals signs.
There was a problem hiding this comment.
TBA doesn't have a avatar API and does not host avatars images. They load them from Base64 every time.
There was a problem hiding this comment.
It actually originally gets them from the FMS API (dracco1993/the-blue-alliance@f9d5cfb). But this wouldn't be an easy task, so you can leave it this way for now.
| @@ -73,11 +73,11 @@ <h1>Developed by <a href="https://www.github.com/FIRSTMap/firstmap.github.io/gra | |||
|
|
|||
| <!-- Javascript Scripts --> | |||
| <script src="data/teamInfo.js"></script> | |||
| <script src="data/teamAvatars.js"></script> | |||
There was a problem hiding this comment.
I feel like we really should move towards using actual JSON files. This was kind of lazy on my part to use a JS file because when I made FIRSTMap I don't think I knew how to run a local web server to make JSON loading possible.
This is a concern for another PR though. I'd be happy to handle that at a later date.
| parsed.district.abbreviation + '</li>' | ||
| } | ||
| if (parsed.week) { | ||
| if (parsed.week) { | ||
| content += '<li><strong>Week:</strong> ' + parsed.week + '</li>' | ||
| } | ||
| var start = new Date(parsed.start_date).toLocaleDateString() |
There was a problem hiding this comment.
This is good work. Thank you for finally making this contribution, I've been wanting to get this implemented for a long time!
I do think we'll need (by no fault of yours) to clean up the code of FIRSTMap a bit though. It's getting kind of spagghetified.
|
I'm just going to clean this up myself so don't bother. |
|
yeah sorry I was at work |
|
After messing around with the map a little bit, I've realized that some markers seem to take a really long time to load while others continue to load instantly. I haven't had the time to look into this deep enough, but my first guess could be due to the two different types of images displayed on the map. Are the FMS team avatars being loaded live or cached locally? If they're not being cached, doing so could probably help speed up the map a lot, especially on slower computers and slower internet connections, especially since the season is over and they shouldn't be changing for a while. And we don't even know if FIRST will necessarily be bringing them back for next season. (Although I hope they do) UPDATE: in researching more, it may be more in part to the vast number of images that are required to be loaded each time the team markers are reloaded. If optimization is desired and/or prioritized, some sort of local caching system might be wise to speed up the site for those who are using it more than once. However, I'm not sure how practical this would be in the grand scheme of things. |
This will display team avatars for teams that haven't manually submitted their logo fixes #130