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 OGP tags for user profiles #228

Merged
merged 3 commits into from
Jan 10, 2022
Merged

Add OGP tags for user profiles #228

merged 3 commits into from
Jan 10, 2022

Conversation

cjreed121
Copy link
Collaborator

This adds OGP tags for profile pages. Here is an example of what it might look like on discord.

image

Coordinator and Project Lead are conditionally shown if that user happens to be those for that semester.

Copy link
Member

@vcfxb vcfxb left a comment

Choose a reason for hiding this comment

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

Some minor changes, but overall this is a good PR. Thank you for writing this.


let mut tags = Tags::default();
tags.title = page_title.clone();
tags.url = format!("/user/{}", id);
Copy link
Member

Choose a reason for hiding this comment

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

Does this URL work without the rcos.io domain at the beginning of it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it does work but I'll go ahead and change it to the same way it is done for meetings (request URI) for consistency.

@@ -87,7 +88,7 @@ async fn profile(
// Return early if there was an error.
// Otherwise we can go forward and check for the user's membership in the RCOS
// Discord server.
return template.in_page(&req, page_title).await;
return template.in_page(&req, page_title.clone()).await;
Copy link
Member

Choose a reason for hiding this comment

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

Why clone here? Ownership will not get moved after returning. Does the compiler error otherwise?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was having some issues with ownership and put that while debugging. After thinking about it, it seems unnecessary since it returns so I'll remove.

@vcfxb vcfxb added enhancement New feature or request Web Interface Issues and PRs related to the Telescope web interface labels Jan 9, 2022
Copy link
Member

@vcfxb vcfxb left a comment

Choose a reason for hiding this comment

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

Looks good!

@vcfxb vcfxb self-assigned this Jan 10, 2022
@vcfxb vcfxb merged commit a44c0dd into master Jan 10, 2022
@vcfxb vcfxb deleted the og_tags_profile branch January 10, 2022 06:14
vcfxb added a commit that referenced this pull request Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Web Interface Issues and PRs related to the Telescope web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants