Census announcement: homepage, /yourschool, and cron support #17562
Conversation
This rapidly-assembled PR contains a numbere of supports for the census announcement. It includes:
- a copy of the hourofcode.com events map for code.org/yourschool, showing schools that have pledged.
- a cronjob and associated updated_pledge_map script which takes each pledge's school ID, looks up its ZIP, and puts a pin on the pledge map.
- homepage support for a special announcement at both desktop and mobile widths
- /yourschool notification support for a special announcement
The final two items are driven by DCDO, and can be activated with something like this:
DCDO.set('census_announcement', {'homepage_text'=> 'This is a special announcement.', 'homepage_subtext' => 'Announcement subtext', 'alert_heading' => 'Announcement heading', 'alert_text'=> "This is the announcement text.", 'alert_url'=> "https://blog.code.org/12345"})
breville
requested review from
Bjvanminnen,
tanyaparker,
Erin007 and
jeremydstone
| alertText={$('#your-school').data("parameters-alert-text")} | ||
| alertUrl={$('#your-school').data("parameters-alert-url")} | ||
| />, | ||
| $('#your-school')[0] |
There was a problem hiding this comment.
Why change this to use jquery?
There was a problem hiding this comment.
I figured if I was going to use jquery for the .data lines, it felt more consistent to use it immediately below. No strong feeling though.
There was a problem hiding this comment.
My own personal feeling is that $(selector)[0] is okay (but still kind of gross) when selector is reasonably complicated, but that not using jquery is preferred when you have an element id. Might mostly be personal preference tho.
There was a problem hiding this comment.
Similarly torn.. just not sure whether it's better to have slightly gross jquery or to mix jquery and native JS in the same function :)
bin/cron/update_pledge_map
Outdated
| tables.authorization = Google::Auth::ServiceAccountCredentials.make_creds( | ||
| scope: 'https://www.googleapis.com/auth/fusiontables', | ||
| json_key_io: StringIO.new(CDO.hoc_map_account.to_json) | ||
| #json_key_io: StringIO.new(CDO.hoc_map_account) |
There was a problem hiding this comment.
old code that can be deleted?
| info_window.close(); | ||
| resizeMap(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
All this JS should be in an entry point rather than in haml.
There was a problem hiding this comment.
Good point. I need to merge this for bugbash tomorrow morning but would like to revisit this. Even better would be to de-dupe this with the (virtually identical) JS for the hourofcode.com map. :/
| = census_announcement['homepage_subtext'] | ||
| %a{href: "/yourschool"} | ||
| %button{style: "margin-top: 20px"} | ||
| Learn more |
There was a problem hiding this comment.
need to be localized? i suspect we already have this string somewhere (or maybe not bc pegasus?)
There was a problem hiding this comment.
This whole announcement for is "en" only, so while it probably is available as a string, it seems a bit simpler to leave it hardcoded. The other strings are coming direct from DCDO so there won't be any localised strings in this piece of UI.
| } | ||
|
|
||
| .legend-title { | ||
| font-size: 18px; |
This rapidly-assembled PR contains a number of supports for the census announcement. It includes:
The final two items are driven by DCDO, and can be activated with something like this:
DCDO.set('census_announcement', {'homepage_text'=> 'This is a special announcement.', 'homepage_subtext' => 'Announcement subtext', 'alert_heading' => 'Announcement heading', 'alert_text'=> "This is the announcement text.", 'alert_url'=> "https://blog.code.org/12345"})Some screenshots
desktop homepage
mobile homepage
/yourschool with notification