-
Notifications
You must be signed in to change notification settings - Fork 942
Game Information / GiantBomb - feature improvements and rename #666
Conversation
fn(); | ||
} | ||
} else { | ||
$.getScript("/js/spice/giant_bomb/more_info/"+encodeURI(items[i].id)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TopHattedCoder I think it would be better if you removed the callback
parameter from spice to
in MoreInfo.pm and Reviews.pm. The you can use $.getJSON
here and avoid the use of defining multiple callback functions, which I think makes the code easier to follow.
@TopHattedCoder Looks like a lot of files came in to this pull request. Is it okay if you make a new one with just BBC in there? Would appreciate that :) |
@jagtalon Haha; will do. |
@TopHattedCoder actually before you spend more time on this PR -- do you happen to know if another API might be able to provide this information? We had a small talk about this PR, trying to decide how it could be improved or re-written because I'm not a fan of this function, https://github.com/duckduckgo/zeroclickinfo-spice/pull/666/files#diff-4472ebbaf58de65bcc1d8e4a8aaea1d2R18. It seems like this will be making an extra 2 requests per relevant item (which right now is limited)? If so this kind of behaviour should really be avoided as it slows down the spice considerably. Would you be willing to look into another source? If not, for now I think the extra API requests should be removed to keep the IA simple and fast. cc// @jagtalon @russellholt |
Fair enough :S. How about one request per game? Then it could still use the
|
…tiple times into closure
@moollaza Okay, I've stripped it of all the AJAX requests and simplified it a ton. Look at the commit history if you want the full story. Here's what it currently looks like: |
@TopHattedCoder this looks awesome! It sucks that the GiantBomb API doesn't send us all the info in one request -- perhaps after launching this we could ask them to help us with the API. Or like I said, maybe we'll have to go elsewhere to get all that awesome meta-data because I really would like to see rating/review data and genre stuff cc// @zekiel @pswam did we have any plans for getting this kind of data? |
brandAndPrice: false, | ||
rating: false, | ||
variant: "poster", | ||
moreAt: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that's already the default for the products_simple
group
@TopHattedCoder I think this might look nice if we mimic the movies/inTheaters look by removing text from the tiles so the images are all that shows? |
@moollaza Good point, I'll remove the title |
We could ask for suggestions on the forum/twitter. Thoughts? @TopHattedCoder is there an associated duck.co/idea for this? Could aggregate the source suggestions there |
thanks @TopHattedCoder --I'll ping out from our channels to try and drive some nominations. |
@TomBebbington Our mistake--we thought that the |
{{#if platforms}} | ||
<span class="detail__callout"> | ||
Available On: | ||
{{#each platforms}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for the platforms
array to have too many elements? Maybe 50 or something that might fill up the page?
@TomBebbington It looks pretty good. :) |
Changed size of the labels (i.e. Active Ingredients, Inactive Ingredients) to 1.15em
So, what's the status on this ? I was planning on making a Spice for this, before seeing it already existed, but nothing triggers. Was this never finished ? I could take over and update it if @TomBebbington doesn't plan to update it anymore ? |
It might take a while to load because you have to get the user reviews for each release of the game. I capped the releases at 5 and made it start from the latest release as that should be the most up-to-date. Also, make sure you have an API key when you're testing it.