Improves snowflake effect #1940
Conversation
| @@ -17,7 +17,7 @@ attribution web => [ 'https://www.duckduckgo.com', 'DuckDuckGo' ], | |||
| github => [ 'https://github.com/duckduckgo', 'DuckDuckGo'], | |||
| twitter => ['http://twitter.com/duckduckgo', 'DuckDuckGo']; | |||
|
|
|||
| spice to => 'http://isitsnowingyet.org/api/check/$1/key/{{ENV{DDG_SPICE_SNOW_APIKEY}}}'; | |||
| spice to => 'https://duckduckgo.com/js/spice/snow/$1'; | |||
There was a problem hiding this comment.
This isn't necessary.
The spice to url is where the actual request is made to. The duckduckgo.com/js/spice/snow/$1 call is re-written via NGXINX to the spice to url. This way the client makes a request to DDG and we in turn make a request to the API to maintain your privacy.
Please switch this back, otherwise the IA will break in production.
|
Made the necessary changes, looks like this now and triggers normally! |
| }, | ||
| onShow: function(){ | ||
| // add the snowflakes, if it's snowing | ||
| if(!api_result.is_snowing) { return; } |
There was a problem hiding this comment.
Nit: the closing brace should be on the line below:
if (...) {
...
} else {
...
}
|
@gokul1794 I'll let the design team decide on a color, but the Navy is definitely more legible. |
|
@moollaza All right :) |
|
Looking great @gokul1794 ! Let's stick with the Gray background color. However, can we change the subtext to # 666? The white text is a bit difficult to read. Any chance to get this up on a test machine to see the animation in action? |
|
@chrismorast, Is this okay? But with the theme below, the main text and the base links are hard to read |
|
@gokul1794 , the default theme version is good. Any chance we can change the BG color for the dark theme to #292929? @moollaza , maybe you can offer some insight here as to the best approach for that. |
|
We need to use the 'tx-clr' and 'bg-clr' classes on the elements in order to make them look good across themes. |
|
They should be covered on the docs under Spice display, variants |
|
@moollaza There's no mention of how to change the background color? |
| @@ -11,6 +11,6 @@ | |||
| font-weight: 900; | |||
| } | |||
|
|
|||
| .zci--snow .zci__header { | |||
| color: inherit; | |||
| .c-base__content { | |||
There was a problem hiding this comment.
whoops, you need to keep the .zci--snow here to namespace the CSS. Otherwise you're potentially going to change all the content on the page that matches your selector:
.zci--snow .c-base__content {
...
}
|
@gokul1794 to target the dark theme in the CSS please prefix your rule with .dark-header .zci--snow .foo {
...
} |
|
@chrismorast, Added, |
|
@moollaza Our stylesheet? |
|
Hey @gokul1794 , one more thing... please use "tx-clr--dk2" as the class for the subtext (No snow is falling, sigh). That way it will auto update to a lighter color for the dark theme. Thank you! |
|
@chrismorast, Is this good enough? |
|
@moollaza Is this ready for merge? |
|
@gokul1794 , yes, that's good. However, I notice now that the notch in the light gray version is not adjusted to match. I'm not sure we've accounted for that in updating the BG color. What would the lighter gray version look like with f2f2f2 as the BG (the default color)? Would it be too subtle? I apologize for the last minute change. |
|
@chrismorast The problem with the default background color is that, the snowflakes aren't very visible! |
|
@gokul1794 I spoke with Chris and we agreed that we're going to revert back to the default background colour for the normal theme, because altering the colour creates issues with the separator in the More at link and the notch in the tab. I'll merge this and update. 👍 BTW, in the future, please make sure you've formatted/indented your code properly. It looks like the indentation is off in a few places here. I'll fix 😃 |
|
@moollaza Sorry about that, I'll make sure it doesn't happen next time. Thanks! 👍 |
|
@gokul1794 its no problem 👍 -- Let your editor do all that hard work -- I have a sublime text package that strips trailing whitespace and another that re-indents the code. Together they do all the heavy lifting 😄 |
|
Continued in #1993 |
This is how it looks like now,
This should fix #1562