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

add SEO tags #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add SEO tags #23

wants to merge 1 commit into from

Conversation

nilinswap
Copy link
Contributor

No description provided.

@amitu amitu changed the title add og_scripts add SEO tags Nov 26, 2019
@@ -10,6 +10,9 @@ pub struct PageSpec {
pub redirect: Option<String>,
#[serde(skip)]
pub rendered: String,
// add struct OGStruct here
// it would have fields title, image, url, type
//
Copy link
Member

Choose a reason for hiding this comment

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

This has to be optional struct. Also for future, such approach PR could have included the commented out actual line instead of instruction.

Copy link
Member

Choose a reason for hiding this comment

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

We also discussed a trait ToSEO with toSEOwith(&self) -> Option<SEO> default implementation (that returns None) for types implementing realm::Page.

Copy link
Member

Choose a reason for hiding this comment

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

Also do a survey of other tags, eg there are twitter specific headers.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can rename this whole thing from being specific to SEO, to make it HTML Meta specific so we can overwrite favicon, css etc also.

pub fn render(&self, is_crawler: bool) -> Result<Vec<u8>, failure::Error> {
let data = escape(serde_json::to_string_pretty(&self)?.as_str());
let mut html = HTML_PAGE.clone();

html = html.replace("__realm_title__", &self.title);
html = html.replace("__realm_title__", &self.title);// add __og_script__, use get_og_scripts function()
Copy link
Member

@amitu amitu Nov 26, 2019

Choose a reason for hiding this comment

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

Write more commented out actual code so we know what parameters are being passed etc. Try to detail out as much without writing function bodies (tho write function body in comment form if needed).

@amitu
Copy link
Member

amitu commented Jan 6, 2020

@nilinswap ping.

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