URW-189 Open Graph tags#179
Conversation
Updated default value setting in head() Minor db connection fix
Kenneth-W-Chen
requested review from
WillowHouchin,
loganthebrewer and
tjcrozier
| @@ -1,6 +1,6 @@ | |||
| <?php | |||
| require('../../template/top.php'); | |||
| head('Merch Ordered', true); | |||
There was a problem hiding this comment.
This change shouldn't be needed, since it's just passing in defaults, right?
There was a problem hiding this comment.
The default description is null which puts the default description. Putting an empty string sets the OG description to be empty.
| //require(BASE . '/api/google/count_majors.php'); | ||
|
|
||
| head('About Us', true); | ||
| head('About Us', true, false, false, "UNT Robotics is a broad engineering student organization at the University of North Texas. We focus on developing student's skills in engineering & robotics, which involves a range of beginner workshops, industry talks, robotics-based hackathons, recreational projects and competitions."); |
There was a problem hiding this comment.
I'm not sure how I feel about having these as function args...
What if we create two new classes:
HeadingConfig which contains the auth, return, and the second class OpenGraphConfig
That way this ugliness of stacking params should be fixed and instead we'll have a nicer way to specify something like
$config = {
auth: false,
og: $og_config
}
There was a problem hiding this comment.
Should it go in template/top.php or in a separate file in template/classes/?
There was a problem hiding this comment.
Each would be its own new class under template/classes and head would take a single argument of $config
There was a problem hiding this comment.
Can you instantiate objects in PHP like that? The only way I can see to get a similar instantiation is if it's an array. As an object, it ends up moving the messy stacking of params into a constructor instead of the head function like this:
$og_config = new OpenGraphConfig('UNT Robotics is a broad engineering...', 'images/og-image-2.png');
$config = new HeadingConfig('About Us', true, true, false, $og_config);
head($config);I don't think that's any better than extracting the params into separate variables and passing the variables instead of values.
There was a problem hiding this comment.
Do some platforms require a square og image? I vaguely recall from my days of setting up og at NT Daily that I had to specify a square image for facebook or twitter
There was a problem hiding this comment.
I think twitter has a specific tag since the rectangle image gets cropped. https://devcommunity.x.com/t/twitter-card-for-open-graph/149336
You can specify the width and height for the image too. With Facebook it helps it load properly apparently? https://developers.facebook.com/docs/sharing/webmasters/#images
I'll try messing around with a square image
| head("Buy {$product->get_name()}", true); | ||
|
|
||
| preg_match("@^(.+?)•@ims", $catalog_product->get_description(), $m); | ||
| //var_dump($m); |
There was a problem hiding this comment.
Remove the debugging code
| require(BASE . '/api/printful/printful.php'); | ||
| require(BASE . '/template/functions/functions.php'); | ||
| require(BASE . '/template/functions/payment_button.php'); | ||
| //require(BASE . '/template/functions/payment_button.php'); |
There was a problem hiding this comment.
Why is this commented out?
There was a problem hiding this comment.
It was throwing an error when I pushed it to the dev site (probably because I deleted the file)
| )); | ||
|
|
||
| $payment_button = new PaymentButton( | ||
| /* $payment_button = new PaymentButton( |
There was a problem hiding this comment.
If any code is commented, it needs to:
- Have a TODO or FIXME comment, with a backlog ticket number to address the issue that led to it being commented out
- Or be considered dead code and just deleted (the nice thing about git is we can still find dead and deleted code from the past)
3 participants
Adds Open Graph tags to every page. These tags are used for embeds on social media and certain messaging platforms, like Facebook and Discord.
This PR adds the required tags. The only optional tag added is
site_name.Also fixes 2 bugs in
/template/top.php:mysqli->connect().authwas set totruedue tonullbeing a falsy value.