Gifs Spice: small code cleanup, ready for launch #1751
Conversation
| total: res.pagination.total_count, | ||
| itemType: 'Gifs' | ||
| }, | ||
| var searchTerm = DDG.get_query().replace(/gifs?/i,'').trim(); |
There was a problem hiding this comment.
This won't match the trigger giphy
| total: res.pagination.total_count, | ||
| itemType: 'Gifs' | ||
| }, | ||
| var searchTerm = DDG.get_query().replace(/gifs?|giphy/ig,'').trim(); |
There was a problem hiding this comment.
Maybe shouldn't be //g? There's the case where you could search for "giphy gifs" and it would strip out both of them.
There was a problem hiding this comment.
trim() looks like it's only on IE 9. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/Trim
There was a problem hiding this comment.
trim() looks like it's only on IE 9.
You deploy a polyfill on production for the trim function 👍
https://duckduckgo.com/d1758.js
There was a problem hiding this comment.
@MrChrisW I should know that--thanks!
|
I actually like the gifs to move in the thumbnails but they eat up too much data. Got 17mb for "hello gif". We should solve the mobile problem on our end though. |
|
I think this one can move ahead then? |
Conflicts: share/spice/gifs/gifs.js
|
@moollaza I added |
Gifs Spice: small code cleanup, ready for launch
This one should be ready now for production. I'm wondering if we should rename the package
DDG::Spice::Giphyor potentially,DDG::Spice::Gifphy::Search-- I'd like to make IA's for their other endpoints so the names would be similar, i.e.Gihpy::Trending,Giphy::Random,Giphy::TranslateI also noticed that our CSS isn't applying the
margin: autoto the images in the detail pane so they're floating off to the left. Not sure if we want to fix that internally or add CSS to this IA?I noticed that on mobile, the detail view uses the fixed image instead of the Gif -- is that expected?
Update: Looking at the template, we use the thumbnail for the detail view on mobile (because we set
loadHighRes = false-- I think we need better handling of that for gifs? No point it showing a static image in the detail, even on mobile.../cc @jagtalon @bsstoner @russellholt