-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
changing templates #46
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. A couple of minor comments and then we'll just want to get @kelockhart and/or @aaccomazzi to confirm they're happy with the look of the new email template.
myadsp/tests/test_utils.py
Outdated
u'<h3><a href="https://ui.adsabs.harvard.edu/search/q=bibstem%3Aarxiv?utm_source=myads&utm_medium=email&utm_campaign=type:general&utm_term=123&utm_content=queryurl" title="" style="text-decoration: none; color: #000000; font-weight: bold;">Query 1</a></h3>') | ||
self.assertIn(u'href="https://ui.adsabs.harvard.edu/abs/2012yCat..51392620N/abstract?utm_source=myads&utm_medium=email&utm_campaign=type:general&utm_term=123&utm_content=rank:1"', split_payload[98]) | ||
|
||
# self.assertIn(u'href="https://ui.adsabs.harvard.edu/abs/2012yCat..51392620N/abstract?utm_source=myads&utm_medium=email&utm_campaign=type:general&utm_term=123&utm_content=rank:1"', split_payload[110]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still want to hold onto this commented line or is it good to delete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a small change to it and uncommented it.
requirements.txt
Outdated
@@ -1,6 +1,6 @@ | |||
adsputils==v1.2.7 | |||
alembic==0.9.1 | |||
psycopg2==2.8.3 | |||
psycopg2-binary==2.8.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if there is a practical difference to the deployment, but it might be worth reverting this back to psycopg2
before the merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
myadsp/templates/email.html
Outdated
@@ -53,8 +53,20 @@ | |||
<td align="center" valign="top" style="font-family:Arial;"> | |||
<table border="0" cellpadding="0" cellspacing="0" width="100%" id="emailBody" style=""> | |||
<tr> | |||
<td align="center" valign="center" background="https://ui.adsabs.harvard.edu/styles/img/background.jpg" style="width:100%; background-color: #150E35; padding: 10px;" > | |||
<p><img src="https://ui.adsabs.harvard.edu/styles/img/ads_logo.png" alt="Astrophysics Data System" style="width: 50%; color: #ffffff; font-size: 34px; font-family: Arial;"/></p> | |||
<td align="center" valign="top" id="m_4393282051944905389m_-5269006104307297584templateHeader" style="background:#222222 url("https://ci3.googleusercontent.com/meips/ADKq_NbWD9eAlbuABElVbFQdmkeLIM77u3maXBPXOELnNNSunHZjJPYgSI3sNbwJe2DflvWFjSsuYrmBmf0c6t9EpPYo4SB_GMN_NbNVPj2-211hCOy4O0Xj__LxqnXB1gY2k6EOThxVlC1c2RaU3FIpIfyzcfXCaAO5l08=s0-d-e1-ft#https://mcusercontent.com/ed0cf26186d0cdd7bad11ed29/images/7d074991-6d2a-8279-c6bf-6150e8d7020c.png") no-repeat center/cover;background-color:#222222;background-image:url(https://ci3.googleusercontent.com/meips/ADKq_NbWD9eAlbuABElVbFQdmkeLIM77u3maXBPXOELnNNSunHZjJPYgSI3sNbwJe2DflvWFjSsuYrmBmf0c6t9EpPYo4SB_GMN_NbNVPj2-211hCOy4O0Xj__LxqnXB1gY2k6EOThxVlC1c2RaU3FIpIfyzcfXCaAO5l08=s0-d-e1-ft#https://mcusercontent.com/ed0cf26186d0cdd7bad11ed29/images/7d074991-6d2a-8279-c6bf-6150e8d7020c.png);background-repeat:no-repeat;background-position:center;background-size:cover;border-top:0;border-bottom:0;padding-top:153px;padding-bottom:153px"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should talk to the UI folks to see if we can host these images in the styles/
directory as we did with the old ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Asked Jennifer.
The header/footer look good to me! |
Agree, looks great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think things are looking good. Excited to see these in action.
I removed any mentions of NASA and replaced ADS with ADS/SciX for now.
The header looks like this (updated):
Footer looks like this: