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

Hearthstone Spice debut #1452

Merged
merged 18 commits into from
Mar 18, 2015
Merged

Hearthstone Spice debut #1452

merged 18 commits into from
Mar 18, 2015

Conversation

Akryum
Copy link
Collaborator

@Akryum Akryum commented Jan 27, 2015

Spice Pull Request

What does your Instant Answer do?
It shows infos about any Hearthstone card with a thumbnail.

What problem does your Instant Answer solve (Why is it better than organic links)?
It allows to get instantly main informations like what a card does, without searching in a database or a wiki.

What is the data source for your Instant Answer? (Provide a link if possible)
Gamepedia Hearthstone Wiki
http://hearthstone.gamepedia.com/

Why did you choose this data source?
It seems to be the most accurate, and has image for every card.

Are there any other alternative (better) data sources?
Not better alternatives:
http://hearthstonejson.com/
http://hearthstoneapi.herokuapp.com/

What are some example queries that trigger this Instant Answer?
hearthstone leeroy
hearthstone tidecaller
hearthstone murloc warleader

Which communities will this Instant Answer be especially useful for? (gamers, book lovers, etc)
Gamers!

Is this Instant Answer connected to a DuckDuckHack Instant Answer idea?
No.

Which existing Instant Answers will this one supersede/overlap with?
None I can think of.

Are you having any problems? Do you need our help with anything?
No thanks. :)

What are the terms of use for the API? Will DuckDuckGo need specific authorization (e.g. an API key)? Are there any costs associated with API usage?
The API is a Mediawiki API. A Warranty disclaimer applies.

Where did you hear about DuckDuckHack? (For first time contributors)
On the duckduckgo timeline (https://duckduckgo.com/about). :)

What does the Instant Answer look like? (Provide a screenshot for new or updated Instant Answers)

duckduckgo-hearthstone3

Checklist

Please place an 'X' where appropriate.

[x] Added metadata and attribution information
[x] Wrote test file and added to t/ directory
[x] Verified that instant answer adheres to design guidelines (https://duck.co/duckduckhack/design_styleguide)
[x] Verified that instant answer adheres to code styleguide (https://duck.co/duckduckhack/code_styleguide)
[x] Tested cross-browser compatibility

    Please let us know which browsers/devices you've tested on:
    - Windows 8
        [] Google Chrome
        [] Firefox
        [] Opera
        [] IE 10

    - Windows 7
        [x] Google Chrome
        [x] Firefox
        [x] Opera
        [] IE 8
        [] IE 9
        [] IE 10
(Tested on IE11)

    - Windows XP
        [] IE 7
        [] IE 8

    - Mac OSX
        [] Google Chrome
        [] Firefox
        [] Opera
        [] Safari

    - iOS (iPhone)
        [] Safari Mobile
        [] Google Chrome
        [] Opera

    - iOS (iPad)
        [] Safari Mobile
        [] Google Chrome
        [] Opera

    - Android
        [x] Firefox
        [] Native Browser
        [x] Google Chrome
        [] Opera

IA Page: https://duck.co/ia/view/hearthstone


{{#if health}}
<span> - Health: <b>{{health}}</b> <image class="zci--hearthstone--icon" src="http://hydra-media.cursecdn.com/hearthstone.gamepedia.com/1/17/Health_icon.png"/></span>
{{/if}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to use the imageProxy helper for all images, see https://duck.co/duckduckhack/spice_handlebars_helpers#imageproxy

@MrChrisW
Copy link
Collaborator

@Akryum This is a great first pass! Thanks for contributing 🍰 I've added some feedback and I'm sure other community leaders will check this out shortly. Feel free to ask any questions. Do you own the API endpoint? bytevortex.net

@Akryum
Copy link
Collaborator Author

Akryum commented Jan 28, 2015

@MrChrisW Thanks for the feedback! And yes I own bytevortex.net ;)

@MrChrisW
Copy link
Collaborator

@Akryum No problem! Happy to help. Great!

@Akryum
Copy link
Collaborator Author

Akryum commented Jan 28, 2015

Feedback applied! :)

@MrChrisW
Copy link
Collaborator

Thanks @Akryum, We'll just need to await a staff review or other feedback from the community!

@Akryum
Copy link
Collaborator Author

Akryum commented Jan 28, 2015

Ironed out some quirks.

@MrChrisW
Copy link
Collaborator

@Akryum 👍

@Akryum
Copy link
Collaborator Author

Akryum commented Jan 28, 2015

Got a question: How are IA supposed to be translated/regionalized?

@MrChrisW
Copy link
Collaborator

@Akryum Good question!

Right now all Instant Answers provide English results. Translation will come later, so initially we're moving forward with just English answers.

@moollaza
Copy link
Member

@Akryum @MrChrisW

This is looking pretty good so far, I have 2 concerns:

  1. We're using the base template and a lot of custom HTML. Let's use a pre-defined template group (I think the info group should work here) and reduce the amount of custom code for this. I think the look/feel of the new MTG Spice https://duckduckgo.com/?q=mtg+nullify&ia=magicthegathering would be ideal. It also makes use of the InfoBox so we have room to show lots of data with minimal effort and templating.

  2. We're not doing any relevancy checking against the query. Right now this will trigger whenever the query has "hearthstone" anywhere in the query. I'm sure a large portion of those potential queries aren't a search for a hearthstone card (e.g "hearthstone game", "hearthstone instructions", "hearthstone stoves")

    Let's improve our triggers and add some checking so we're confident the query that triggered this is likely to be a search for a hearthstone card and then we don't have to worry about showing lots of potentially irrelevant results.

// Card Data
// May (but very unlikely) return <undefined> values for : image_url, Has_card_type, Has_rarity,
// Has_class, Has_mana_cost
var card = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be handled with a normalize function: https://duck.co/duckduckhack/spice_displaying#data-normalization

@Akryum
Copy link
Collaborator Author

Akryum commented Jan 28, 2015

@moollaza I don't really think of a good way of restricting the trigger... A keyword blacklist maybe?

@Akryum
Copy link
Collaborator Author

Akryum commented Jan 28, 2015

Using info template:
duckduckgo-hearthstone4

@Akryum
Copy link
Collaborator Author

Akryum commented Jan 28, 2015

I think that duckpan test modifies my .travis.yml all the time. :/

@abeyang
Copy link
Contributor

abeyang commented Feb 5, 2015

@Akryum can you throw this onto DDH1? Would like to see this design with other cards (esp. those with a fuller description). But I think we're almost there!

@Akryum
Copy link
Collaborator Author

Akryum commented Feb 5, 2015

@abeyang Well, you may ask @jagtalon for that I think. :3

@jagtalon
Copy link
Member

jagtalon commented Feb 6, 2015

@jagtalon
Copy link
Member

jagtalon commented Feb 6, 2015

@abeyang I don't think we have #999 as a standard color in https://duckduckgo.com/styleguide. What about using the tx-clr--lt class which adds #888?

screen shot 2015-02-06 at 3 49 01 am

@Akryum
Copy link
Collaborator Author

Akryum commented Feb 6, 2015

Flavor text now has tx-clr--lt css class.

@moollaza
Copy link
Member

moollaza commented Feb 7, 2015

@Akryum thanks!

@jagtalon can you update the code on DDH1 please? and then @abeyang can take another look and sign-off.

@jagtalon
Copy link
Member

@moollaza
Copy link
Member

Looks great! Would like a design sign-off and then we can merge it in 👍

@abeyang
Copy link
Contributor

abeyang commented Feb 20, 2015

Hi @Akryum , after looking at a few examples, I think it's best to just fit the little "gag" into the subtitle. For example, the Crackle card would look more like this:

Crackle
Snap! This card! Pop!

Deal 3-6 damage. Overload: (1)

Hope that makes sense. Let us know if you need help with anything!

@moollaza
Copy link
Member

@abeyang unfortunately the Info template doesn't allow for a subtitle property.

We could add the markup to the description template, but unless we add additional CSS to shift it on the page it's going to look a little odd because the subtitle div will be contained in the zci__content div that wraps the description.

I think this has been fixed in the updated templates, so perhaps we can leave it as-is and change it later?

@chrismorast
Copy link
Contributor

He means subheader @moollaza
screen shot 2015-02-20 at 3 51 59 pm

@Akryum
Copy link
Collaborator Author

Akryum commented Feb 21, 2015

@chrismorast Do I put the flavor text subheader div in the content template or is there any property for that?

@chrismorast
Copy link
Contributor

Hi @Akryum , put that content in the subtitle property. We will be deploying an update to the template to allow for this within the next day or two.

@moollaza
Copy link
Member

@Akryum sorry! I didn't realize you had updated this -- we don't get notified of commits, only comments!

@moollaza
Copy link
Member

LGTM 👍

moollaza added a commit that referenced this pull request Mar 18, 2015
@moollaza moollaza merged commit d253de4 into duckduckgo:master Mar 18, 2015
@moollaza
Copy link
Member

@Akryum I just noticed we're not using the CSS file -- can you make a PR to remove it?

@moollaza
Copy link
Member

Nevermind I see what's happening. The text comes as HTML from the API. Probably best to use DDG.stripHTML() but this is fine.

@moollaza
Copy link
Member

moollaza commented Apr 1, 2015

@Akryum congrats! It's live: https://duckduckgo.com/?q=hearthstone+murloc+warleader&ia=hearthstone

Thanks again for taking the time to contribute! We really appreciate it. Feel free to stick around and comment/help on other PR's & Issues or even submit more Instant Answers!

@Akryum
Copy link
Collaborator Author

Akryum commented Jun 14, 2015

Hi! It seems that the card IA is no longer working. Is there any problem I can help with?

@moollaza
Copy link
Member

@Akryum do you have any examples? I just tried and it worked:

hearthstone_murloc_warleader_at_duckduckgo_and_duckduckgo_-_hipchat

@Akryum
Copy link
Collaborator Author

Akryum commented Jun 16, 2015

@moollaza It seems that some cards no longer display the IA like these:
https://duckduckgo.com/?q=hearthstone+blizzard
https://duckduckgo.com/?q=hearthstone+oasis
https://duckduckgo.com/?q=hearthstone+boom+bot

I just tested my api endpoint and it get quite slow sometimes. Has the IA a timeout? (I will see what I can do to speed up the api.)

@Akryum
Copy link
Collaborator Author

Akryum commented Jun 16, 2015

I have added a cache system to greatly speed up calls to my api so the IA works again after a few try (when the card cache is ready).

@moollaza
Copy link
Member

@Akryum yes there's a default timeout of 5000ms.

Your 3 examples all worked for me, so I guess everything is good now?

@Akryum
Copy link
Collaborator Author

Akryum commented Jun 18, 2015

Yes, evrything is working again :)

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

Successfully merging this pull request may close these issues.

7 participants