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

feat: add rich previews #676

Merged
merged 10 commits into from
Oct 26, 2023
Merged

feat: add rich previews #676

merged 10 commits into from
Oct 26, 2023

Conversation

CharlesSheelam
Copy link
Contributor

Utilized the open graph protocol to add rich previews for Base page, codebases, events, and jobs.

@CharlesSheelam CharlesSheelam changed the title Feature: add rich previews feat: add rich previews Oct 13, 2023
Copy link
Member

@alee alee left a comment

Choose a reason for hiding this comment

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

Good work @CharlesSheelam ! This sets up the groundwork nicely. A few minor nitpicks and suggested edits included inline.

We should also identify other "important" pages to put these OGP tags in as well...

<meta property="og:type" content="website">
<meta property="og:title" content="CoMSES Net">
<meta property="og:description" content="CoMSES Net is an international open research community dedicated to fostering good practices for computational / agent based modeling.">
<meta property="og:image" content="https://complexity.asu.edu/sites/default/files/comses.jpg">
Copy link
Member

Choose a reason for hiding this comment

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

these probably should refer to an internal static image.. I guess the current logo is generated by JS but it could live in core/static/images as well and be accessible via something like {{ static("images/comses-logo.png") }}.

This can be done by copying frontend/src/assets/images/comses-logo.png to django/core/static/images and then replace all og|twitter:image references to the static invocation above

<!-- Facebook Meta Tags -->
<meta property="og:url" content="https://www.comses.net/">
<meta property="og:type" content="website">
<meta property="og:title" content="CoMSES Net">
Copy link
Member

Choose a reason for hiding this comment

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

should we include the full expanded acronym here, Network for Computational Modeling in the Social and Ecological Sciences or will it be too long for the rich preview?

Copy link
Contributor Author

@CharlesSheelam CharlesSheelam Oct 16, 2023

Choose a reason for hiding this comment

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

We could test the full expanded acronym, I have changed it to the full thing for now. If it seems to long later I can change it back.

<meta property="og:type" content="website">
<meta property="og:title" content="Community Jobs">
<meta property="og:description" content="View all jobs related to CoMSES">
<meta property="og:image" content="https://complexity.asu.edu/sites/default/files/comses.jpg">
Copy link
Member

Choose a reason for hiding this comment

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

should we create custom images for comses jobs / events / codebases and add them to static? might be a nice UX touch

<meta property="og:url" content="{{ request.build_absolute_uri(absolute_url) }}">
<meta property="og:type" content="article">
<meta property="og:title" content="{{ codebase.title }}">
<meta property="og:description" content="{{ codebase.description.raw|truncate(150) }}">
Copy link
Member

Choose a reason for hiding this comment

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

we should probably use codebase.summarized_description (and modify its implementation to make it work better for this use case as needed)

<meta property="og:url" content="{{ request.build_absolute_uri(absolute_url) }}">
<meta property="og:type" content="article">
<meta property="og:title" content="{{ title }}">
<meta property="og:description" content="{{ description }}">
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 this is a copypasta issue, for MemberProfile this should probably be name and bio ?

<meta property="og:url" content="{{ request.build_absolute_uri(absolute_url) }}">
<meta property="og:type" content="article">
<meta property="og:title" content="{{ title }}">
<meta property="og:description" content="{{ description }}">
Copy link
Member

Choose a reason for hiding this comment

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

probably best to use summary instead, as description can be quite long

same thing with Events

Copy link
Member

@alee alee left a comment

Choose a reason for hiding this comment

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

LGTM! Just a minor note to replace the og:image /static/image URL with the jinja static() function instead

<meta property="og:type" content="website">
<meta property="og:title" content="The Network for Computational Modeling in the Social and Ecological Sciences (CoMSES Net)">
<meta property="og:description" content="CoMSES Net is an international open research community dedicated to fostering good practices for computational / agent based modeling.">
<meta property="og:image" content="/static/images/logo.png">
Copy link
Member

Choose a reason for hiding this comment

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

refer to static assets in a jinja template using the incantation I shared earlier,

{{ static("images/logo-comses.png") }}

so the full line would be something like

<meta property="og:image" content="{{ static('images/logo-comses.png') }}">

For more info on why we dynamically generate these static links,

https://docs.djangoproject.com/en/4.2/howto/static-files/#configuring-static-files

(though the syntax looks a little different since we are using Jinja templates instead of Django templates)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies, forgot to fix these. I just updated them and the pull request is updated.

@alee alee merged commit 8f66ed5 into comses:main Oct 26, 2023
4 checks passed
@CharlesSheelam CharlesSheelam deleted the new_rich_preview branch February 5, 2024 23:15
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