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

Feature/rails seo #3319

Merged
merged 17 commits into from
Feb 22, 2018
Merged

Feature/rails seo #3319

merged 17 commits into from
Feb 22, 2018

Conversation

edbrett
Copy link
Contributor

@edbrett edbrett commented Feb 22, 2018

Overview

The country pages (and the whole app) needed a little SEO love. So we gave it. We are giving people. We give. To give is to gave.

For this feature we have updated the SEO capabilities of the country page to include:

  • Dynamic country fetch (courtesy of @simaob) to find the english country name from the ISO in the model.
  • Server side dynamic rails SEO for both page sharing and widget sharing. Now if you share the page you recieve a title of CountryName | GFW (ish) and if you share a widget the social card reads Widget Names in CountryName | GFW. Pretty fancy eh?!
  • Updated rails structure to allow react based layouts for embeds and pages without repeated code.
  • General tidy of rails layouts and views

Demo

Try sharing stuff with twitter.
screen shot 2018-02-22 at 11 32 57

Notes

We still need to get this working with dynamic widget images. To plan is to use dom-to-image to allow use to snap shot the image and store it in an amazon bucket, where we will store it for 30 days or so.

Testing

Going to put on staging and get posting.

@edbrett edbrett requested a review from simaob February 22, 2018 10:42
@edbrett edbrett temporarily deployed to gfw-nav February 22, 2018 10:51 Inactive
Copy link
Contributor

@simaob simaob left a comment

Choose a reason for hiding this comment

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

Small change requested. Other than that it looks good!

@desc = 'Data about forest change, tenure, forest related employment and land use'
def show
@title = @country["name"]
@widget = params[:widget]
Copy link
Contributor

Choose a reason for hiding this comment

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

if you are not using a variable on the view you don't need to make it a instance variable @variable you can use just variable. Or in this case you can just use params[:widget]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok will remove! Thanks for the review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed :)

@simaob simaob merged commit cd59de6 into develop Feb 22, 2018
@simaob simaob deleted the feature/rails-seo branch February 22, 2018 18:52
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.

2 participants