EOY: Load 'disruptive' snippet code into snippet service admin



Mozilla Foundation Communications
4 years ago
3 years ago


(Reporter: andreawood, Assigned: mkelly)


(Blocks: 2 bugs)


(Whiteboard: [EOYFR2014][snippet][p1][dec28])


(2 attachments, 1 obsolete attachment)



4 years ago
Once Foundation dev team finalizes code and it is through review, the snippet needs to be loaded into snippet service. (that's this step*) All the steps:

1. Design snippet (normally Sabrina Ng)
2. Build snippet (incl. review) - (could be thecount or gvn)
*3. Implement in snippet service (normally mkelly)
4. Launch into selected Locales (normally frios)
5. Launch metrics tracking (normally adamlofting)


4 years ago
Blocks: 1112347


4 years ago
Blocks: 1112349

Comment 1

4 years ago
I'm on PTO all next week (except for the holidays, but, well, they're holidays :P), giorgos might be able to cover if you're expecting a code review or someone to add a template to prod in that timeframe. NEEDINFOing him for his availability. :D

(CCing bensternthal just so he's aware of possible work during a week when most of our team is MIA)
Flags: needinfo?(giorgos)
I'll be available on the 22nd and 23rd, possibly on the 26th if needed. Please needinfo me with requests :)
Flags: needinfo?(giorgos)
Created attachment 8540375 [details]

Not sure about the process yet, but this is a disruptive snippet ready for staging.

These are the test values I have been using: https://github.com/ScottDowne/eoy-snippets/blob/master/gulpfile.js#L49-L59

I am happy to answer any questions or concerns.
Attachment #8540375 - Flags: review?(giorgos)
Created attachment 8540944 [details]


Fixed the background image.

There were also changes to the button animation that needed to be made.

Staging link here: https://snippets.allizom.org/show/76/
Attachment #8540375 - Attachment is obsolete: true
Attachment #8540375 - Flags: review?(giorgos)
Attachment #8540944 - Flags: review?(giorgos)
Comment on attachment 8540944 [details]

Looks good. I submitted a pull request [0] against your repo with a small change. Please pull request against mozilla/snippets with the final code.

Also is this really needed? https://github.com/ScottDowne/eoy-snippets/blob/master/simple-snippet/snippet.html#L134

[0] https://github.com/ScottDowne/eoy-snippets/pull/5
Attachment #8540944 - Flags: review?(giorgos) → review+
- Reviewed template code, did some minor changes (== included snippet-id in keyframes)
- Tested with Foxes down to version 26
- Looks good and works good.
- Updated production template "[Fx26+] Mofo End of Year Donation Chooser" [0] with the latest template code. 

One thing to be aware when choosing a background color:
 - Background color *persists*, i.e. when users reload (F5) the page to get a new snippet, the new snippet will appear with the altered background color. Maybe that's a non-issue because I guess that not so many users reload about:home, but maybe you want to avoid that.

[0] https://snippets.mozilla.com/admin/base/snippettemplate/26/
Yeah, without this line: https://github.com/ScottDowne/eoy-snippets/blob/master/simple-snippet/snippet.html#L134

The bottom menu was transparent and I needed to ensure the background colour didn't show through making the icons hard to read.
About background persisting, it's likely less of an issue because this snippet has 100% traffic in US.

So if you refresh you would get the same snippet anyway.

I'm curious as to why it persists though.

Comment 9

3 years ago
(In reply to Scott [:thecount] Downe from comment #8)
> About background persisting, it's likely less of an issue because this
> snippet has 100% traffic in US.
> So if you refresh you would get the same snippet anyway.
> I'm curious as to why it persists though.

It persists because snippet code is always injected into the page even if a snippet isn't visible (see https://abouthome-snippets-service.readthedocs.org/en/latest/overview.html for how snippets are loaded). Thus, your CSS for altering the background of the body is always applied.

General workaround for this is to add a class to the body when the show_snippet event is triggered.
Ah, that makes sense.

Yeah, I'll add that and hopefully there is still time to ship it.
Created attachment 8541818 [details]

Just to be safe, I think we should do what Osmose is suggesting with a class on the body for the background image.

This is the diff for readability sake. https://github.com/ScottDowne/eoy-snippets/commit/99d2ab593f57635c4b6942153c3366f17edda6d8

I updated the staging link with those changes: https://snippets.allizom.org/show/76/

Thanks for the guidance with this, it is appreciated!
Attachment #8541818 - Flags: review?(giorgos)
Created the pull request to mozilla/snippets.


I wasn't sure if I should include the changes in comment #12 so I left them out for now.
Aaand updated production. 

Scott, yes please do include the latest background changes, thnx!
Sweet, thanks so much!

I updated the pull request with the background stuff: https://github.com/mozilla/snippets/pull/28
Attachment #8541818 - Flags: review?(giorgos) → review+
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.