-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
URW-189 Open Graph tags #179
base: develop
Are you sure you want to change the base?
Changes from 3 commits
326a135
a860ec5
8447d6a
1490b49
90cf2c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Kenneth-W-Chen marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
<?php | ||
require('../../template/top.php'); | ||
head('Merch Ordered', true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change shouldn't be needed, since it's just passing in defaults, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default description is |
||
head('Merch Ordered', true, false, false, ""); | ||
|
||
$log = var_export($_REQUEST, true); | ||
$log .= var_export($userinfo, true); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
require('../template/top.php'); | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this commented out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was throwing an error when I pushed it to the dev site (probably because I deleted the file) |
||
|
||
$printfulapi = new PrintfulCustomAPI(); | ||
|
||
|
@@ -61,15 +61,22 @@ | |
$mockup_file = ""; | ||
} | ||
$back_file = $selected_variant->get_file_by_type(PrintfulVariantFilesTypes::BACK); | ||
|
||
head("Buy {$product->get_name()}", true); | ||
|
||
preg_match("@^(.+?)•@ims", $catalog_product->get_description(), $m); | ||
//var_dump($m); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the debugging code |
||
$description = ""; | ||
if (count($m)) { | ||
$description = trim($m[1]); | ||
} | ||
|
||
head("Buy {$product->get_name()}", true, false, false, $description, $mockup_file->get_url() ? $mockup_file->get_url() : $mockup_file->get_preview_url()); | ||
$category_name = strtolower(preg_replace('@^.*\(([^()]+)\)$@i', '$1', $product->get_name())); | ||
if($category_name !== 'gear' && $category_name[-1]!=='s'){ | ||
$category_name .= 's'; | ||
} | ||
|
||
} else { | ||
head("Invalid Product", true); | ||
head("Invalid Product", true, false, false, null, null); | ||
} | ||
|
||
function get_variant_variant($variant_name) { | ||
|
@@ -248,12 +255,6 @@ class="btn variant-btn" | |
?> | ||
|
||
<?php | ||
preg_match("@^(.+?)•@ims", $catalog_product->get_description(), $m); | ||
//var_dump($m); | ||
$description = ""; | ||
if (count($m)) { | ||
$description = trim($m[1]); | ||
} | ||
preg_match_all("@• (.+)\n@i", $catalog_product->get_description(), $m); | ||
//var_dump($m); | ||
$other_info = array(); | ||
|
@@ -289,7 +290,7 @@ class="btn variant-btn" | |
'variant' => $selected_variant->get_id() | ||
)); | ||
|
||
$payment_button = new PaymentButton( | ||
/* $payment_button = new PaymentButton( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If any code is commented, it needs to:
|
||
$product->get_name(), | ||
$product->get_product_price() | ||
); | ||
|
@@ -303,7 +304,7 @@ class="btn variant-btn" | |
$product->get_name(), | ||
get_variant_variant($selected_variant->get_name()) | ||
)); | ||
$payment_button->set_complete_return_uri('/merch/buy/complete'); | ||
$payment_button->set_complete_return_uri('/merch/buy/complete');*/ | ||
|
||
/* | ||
$button = payment_button( | ||
|
@@ -323,16 +324,7 @@ class="btn variant-btn" | |
*/ | ||
|
||
//echo $button['btn']; | ||
$button = $payment_button->get_button(); | ||
if ($button->error === false) { | ||
echo "button success"; | ||
echo $payment_button->get_button()->button; | ||
} else { | ||
// TODO: Alert | ||
?> | ||
<div class="alert alert-danger">An error occurred loading the payment button...</div> | ||
<?php | ||
} | ||
|
||
?> | ||
</div> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 classOpenGraphConfig
That way this ugliness of stacking params should be fixed and instead we'll have a nicer way to specify something like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it go in
template/top.php
or in a separate file intemplate/classes/
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each would be its own new class under
template/classes
andhead
would take a single argument of$config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
I don't think that's any better than extracting the params into separate variables and passing the variables instead of values.