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

6840 : meta tags update #6998

Merged
merged 17 commits into from
Aug 18, 2016
Merged

6840 : meta tags update #6998

merged 17 commits into from
Aug 18, 2016

Conversation

dhovart
Copy link
Contributor

@dhovart dhovart commented Aug 17, 2016

Fixes #6840

  • Add opengraph and general meta tags to posts, profile and tags views.
  • Added a few methods to retrieve the meta data to the existing Person/Post presenter and created a TagPresenter (would like to get some feedback on my approach: I hope it makes sense for you to have a presenter for a Stream::Tag, just the same as we have presenters for ActiveRecord models).
  • Introduced a MetaDatahelper
  • Moved the hardcoded contents already set for meta tags to config/defaults.yml

Hope I'm following your styleguides correctly and that my tests are good enough!

# licensed under the Affero General Public License version 3 or later. See
# the COPYRIGHT file.

class TagPresenter < BasePresenter
Copy link
Member

Choose a reason for hiding this comment

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

TagPresenter to me sounds like being for a single Tag, how about TagStreamPresenter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Would be better indeed.

@cmrd-senya
Copy link
Member

you must have meant #6840

og_profile_username: { property: 'og:profile:username' , content: name },
og_profile_firstname:{ property: 'og:profile:first_name', content: first_name },
og_profile_lastname: { property: 'og:profile:last_name' , content: last_name }
}
Copy link
Member

Choose a reason for hiding this comment

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

You can use the bio as a description here if public_details?, else not.

@dhovart dhovart changed the title 6480 : meta tags update 6840 : meta tags update Aug 17, 2016
@dhovart
Copy link
Contributor Author

dhovart commented Aug 17, 2016

Woops, yes, that was supposed to be 6840. Sorry.
Edited.

og_image: {property: "og:image", content: images},
og_description: {property: "og:description", content: description},
og_article_tag: {property: "og:article:tag", content: tags},
og_article_author:

Choose a reason for hiding this comment

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

Align the elements of a hash literal if they span more than one line.

# licensed under the Affero General Public License version 3 or later. See
# the COPYRIGHT file.

class StreamTagPresenter < BasePresenter
Copy link
Member

Choose a reason for hiding this comment

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

I think I prefer TagStreamPresenter, StreamTagPresenter sounds like it's for a special kind of tag, a "stream tag", while it's a special kind of stream, a "tag stream".

@jhass jhass added this to the 0.6.0.0 milestone Aug 18, 2016
@jhass jhass merged commit bcace2d into diaspora:develop Aug 18, 2016
@jhass
Copy link
Member

jhass commented Aug 18, 2016

Thank you very much!

jhass added a commit that referenced this pull request Aug 18, 2016
SuperTux88 added a commit to SuperTux88/diaspora that referenced this pull request Aug 21, 2016
* fixed old reshares of reshares
* fixed deleted root of a reshare
denschub added a commit that referenced this pull request Aug 22, 2016
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.

4 participants