Add dynamic OGP tags #218
Add dynamic OGP tags #218
Conversation
|
Thank you for the tip with ngrok btw :) |
| @@ -332,7 +332,7 @@ impl TelescopeError { | |||
| // Render the form. | |||
| let page_content: String = form.render()?; | |||
| // Put it in a page. | |||
| return page::with_content(req, form.page_title, page_content.as_str()) | |||
| return page::with_content(req, form.page_title, page_content.as_str(), None) | |||
There was a problem hiding this comment.
adding additional parameters like this now and in the future makes functions polyadic, and by extension, hard to read, i think we should start passing a list of necessary values for page generation, with ogp tag values as the first element, and any future values as additional elements to reduce clutter in function signatures and calls
There was a problem hiding this comment.
Sure that sounds great. Are you thinking of just new struct that contains any additional piece of data? And any non mandatory field or something that can use a default type could be an Option<>? I'm not super familiar with Rust so I'm not sure if this is a good way to go about this. Maybe @alfriadox can weigh in here as well.
There was a problem hiding this comment.
A struct with default fields would definitely not be the wrong answer here, but I am not sure what exactly would be the right one. There are a few options. One is a struct, which would have to implement Default so that we could omit some of the fields during initialization as necessary. Another would just be a json object that gets passed to the template. Creating these with macros is really easy and we would have more flexibility, but we would also loose the benefits of the type system.
If we make changes in how we add dynamic content to pages we should also apply this change to the way navbars are created, since they are also dynamic and work differently than ogp tags currently.
I may make a branch and work on refactoring all of this into a struct, since that's probably our best option.
There was a problem hiding this comment.
i am not familiar with rust so i will let nia take over on this one
| @@ -70,7 +71,16 @@ impl Template { | |||
| req: &HttpRequest, | |||
| title: impl Into<Value>, | |||
| ) -> Result<Template, TelescopeError> { | |||
| page::of(req, title, self).await | |||
| Self::render_into_page_with_tags(self, req, title, None).await | |||
| @@ -63,7 +63,7 @@ impl Responder for FormTemplate { | |||
| let rendered: String = self.render()?; | |||
|
|
|||
| // Put it in a page. | |||
| page::with_content(&req, self.page_title, rendered.as_str()) | |||
| page::with_content(&req, self.page_title, rendered.as_str(), None) | |||
89df20c to
8a8c576
Compare
This PR adds support for dynamic OGP tags and implements them for meetings. Other pages could do something similar to get custom static OGP or dynamic OGP tags. For testing locally, I would recommend using ngrok so Discord can read the page and display the embed.
Here is an example of what a dynamic OGP tags on Discord would look like (a description and host would also show if either were provided):
