Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Game Information / GiantBomb - feature improvements and rename #666

Closed
wants to merge 16 commits into from
Closed

Game Information / GiantBomb - feature improvements and rename #666

wants to merge 16 commits into from

Conversation

TomBebbington
Copy link
Contributor

  • Rename to VideoGames
  • Add several sorting methods and relevancy info
  • Add release date under the heading
  • Use product template

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.

fn();
}
} else {
$.getScript("/js/spice/giant_bomb/more_info/"+encodeURI(items[i].id));
Copy link
Member

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.

@jagtalon
Copy link
Member

jagtalon commented May 7, 2014

@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 :)

@TomBebbington
Copy link
Contributor Author

@jagtalon Haha; will do.

@moollaza
Copy link
Member

moollaza commented May 7, 2014

@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

@TomBebbington
Copy link
Contributor Author

Fair enough :S. How about one request per game? Then it could still use the
products template after putting the rating text as the game's genre.
On 7 May 2014 23:35, "Zaahir Moolla" notifications@github.com wrote:

@TopHattedCoder https://github.com/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 https://github.com/jagtalon @russellholthttps://github.com/russellholt


Reply to this email directly or view it on GitHubhttps://github.com//pull/666#issuecomment-42492100
.

@TomBebbington
Copy link
Contributor Author

@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:
Latest version

@moollaza
Copy link
Member

@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
Copy link
Member

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

@moollaza
Copy link
Member

@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?

@TomBebbington
Copy link
Contributor Author

@moollaza Good point, I'll remove the title

@zekiel
Copy link
Member

zekiel commented May 11, 2014

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

@TomBebbington
Copy link
Contributor Author

@zekiel
Copy link
Member

zekiel commented May 12, 2014

thanks @TopHattedCoder --I'll ping out from our channels to try and drive some nominations.

@yegg yegg closed this May 30, 2014
@jagtalon
Copy link
Member

@TomBebbington Our mistake--we thought that the bttf branch wasn't being used anymore. Please make a PR to master. :)

{{#if platforms}}
<span class="detail__callout">
Available On:
{{#each platforms}}
Copy link
Member

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?

@jagtalon
Copy link
Member

@TomBebbington It looks pretty good. :)

medicosconsultants added a commit to medicosconsultants/zeroclickinfo-spice that referenced this pull request Oct 8, 2014
Changed size of the labels (i.e. Active Ingredients, Inactive Ingredients) to 1.15em
gokul1794 added a commit to gokul1794/zeroclickinfo-spice that referenced this pull request Jun 23, 2015
@ghost
Copy link

ghost commented Dec 3, 2015

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 ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants