Skip to content
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

Update code.org og tags to match the marketing matrix #19398

Merged
merged 3 commits into from Nov 27, 2017

Conversation

joshlory
Copy link
Contributor

theme: responsive_wide
banner: null
layout: wide
video_player: true
social:
"og:title": "<%= I18n.t(:og_title) %>"
"og:description": "<%= I18n.t(:og_description)%>"
"og:image" : "<%= 'https://' + request.host + '/images/fit-1220/social-media/' + share_image %>"
"og:image:width": '1220'
"og:image:height": '640'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used 1220x640 to match the defaults, not sure if that's right.

metadata['og:image'] = CDO.code_org_url('/images/default-og-image.png', 'https:')
metadata['og:image:width'] = 1220
metadata['og:image:height'] = 640

<%
title = hoc_s(:meta_tag_og_title)
title = I18n.t(:og_title_soon) if DCDO.get('soon-hoc', false)
title = I18n.t(:og_title_here) if DCDO.get('actual-hoc', false)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok to mix & match hoc_s and I18n.t here? What's the reason for having two different pathways?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it's fine - as far as I know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hoc_s was added for Tutorial Explorer (#11714) so that we could reuse the same strings on /learn between all of the pegasus domains. Looks like it's become more widely used since then. It makes sense if it's a string that's shared between hourofcode.com and code.org/csedweek.org.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I'm wondering whether it's necessary to use it on pages that are exclusively on hourofcode.com.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tanyaparker tells me that hoc_s pulls from the right place even when pages are exclusively on hourofcode.com. It makes for a simpler translation pipeline. cc @Hamms

@joshlory
Copy link
Contributor Author

@breville PTAL!

title: #HourOfCode Challenge
theme: responsive
style_min: true
social:
"og:title": "Celebrity Challenge"
"og:description": "Win a celebrity video chat for your class!"
"og:image" : "<%= 'https://' + request.host + '/images/social-media/hoc-student-challenge.png' %>"
"og:image" : "<%= 'https://' + request.host + '/images/fit-1920/social-media/' + share_image %>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1920 seems really large for a Twitter/OG image. Facebook seems to recommend min 1200 x 630 and Twitter 1024 x 512 pixels. @tanyaparker may know more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do keep it 1920, are both the images at least 1920?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! I added this because both images are 2mb+ 😕. We can go smaller, but I added an upper bound to match the existing og:image:width size.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully production will serve them smaller than 2mb+ when we use fit-1920 and serve as JPGs?

share_image = hoc ? 'celeb-challenge.jpg' : 'default-og-image.jpg'
description = hoc ? I18n.t(:og_description_celeb) : I18n.t(:og_description)

title = I18n.t(:og_title)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these strings require a content scoop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@breville
Copy link
Member

Longer term, I wonder if we should centralize the definition of our social metadata matrix, in an asset that kind of matches the gsheet-based documentation.

@breville
Copy link
Member

@tanyaparker will review that the items match the matrix correctly. Everything else lgtm.

@breville
Copy link
Member

breville commented Nov 27, 2017

This change can be tested on staging if robots.txt is manually changed to disable the production-only reference.

Disallow: <%= '/' unless rack_env == :production %>

Note that we discovered an issue after deploying last night in which Twitter, unlike Facebook, doesn't offer a manual way to flush its cache of the page's metadata.

https://developer.twitter.com/en/docs/tweets/optimize-with-cards/guides/troubleshooting-cards

I updated my site meta tags, but my Tweet shows the old Card. How do I refresh the Card? Our web crawlers re-index the Card tag information on your page roughly every week.

We tried the bit.ly workaround to no avail. Fortunately, a couple hours later the cache did get flushed, but we don't know of anything we specifically did to make that happen.

@joshlory
Copy link
Contributor Author

We should be able to check OG tags via http://opengraphcheck.com/ on Staging.

@joshlory
Copy link
Contributor Author

Approved by War Room triage for 11/27.

@joshlory joshlory merged commit 2fcc434 into staging Nov 27, 2017
@joshlory joshlory deleted the hoc-og-image-matrix branch November 27, 2017 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants