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

[#39] Auction index #103

Merged
merged 8 commits into from
Dec 19, 2023
Merged

[#39] Auction index #103

merged 8 commits into from
Dec 19, 2023

Conversation

nathan-schmidt-viget
Copy link
Contributor

@nathan-schmidt-viget nathan-schmidt-viget commented Dec 18, 2023

Summary

This sets up the basic audition index page. It set up a template that pulls in a pattern to control the layout of the page.
We filter the images to use the featured image for the Auction if it's set. If not use the featured product image for the rewards Product associated with the Auction. If no featured product, falls back to the WooCommerce default image.

The Auction index gets added as a page which auto adds it to the default menu. (we may need to adjust this a bit, but the only way I could find in the newest WP version to update the menu dynamically)

I am setting the Auctions index to only show 9 posts per page, this is not setting the global # posts in the admin (used for blog index), but just changes on the Auction index.

Issues

Testing Instructions

Auction Menu

  • Create a new site
  • Go to the site dashboard and make sure we have an Auctions page in the pages section
  • Go to the new site and make sure that the first menu item on the site is Auctions

Image Testing

  • Go to a nonprofit site
  • Go to the Auction index page at /auctions/
  • Make sure it is pulling in the auction posts
  • Make sure it is pulling in the right images
    • If the auction does not have a featured image it pulls from the Reward featured image
    • If no Reward featured image it defaults to the WooCommerce image

Post Limit to 9

  • Create more than nine auctions
  • Make sure that the Auction index shows only the first nine.

Screenshots

Index

Screenshot 2023-12-18 at 1 56 20 PM

Default pagination

Screenshot 2023-12-18 at 1 58 27 PM

@clatwell
Copy link
Contributor

This is looking great! Do you mind tackling some quick Nonprofit Site Header polish as part of this PR?

  • When a new Nonprofit Site is created, don't include Cart, Checkout, or Shop page links in the header by default
  • When a new Nonprofit Site is created, include a link to Our Auctions in the header by default

Thank you!!

@nathan-schmidt-viget
Copy link
Contributor Author

nathan-schmidt-viget commented Dec 18, 2023

@clatwell Totally, I had gotten the first part of the ticket completed and wanted to get the doc/images into a draft before I did the header link and # of posts. I just got both of these added.

I will have to look into removing the default WooCommerce pages in the header

@nathan-schmidt-viget
Copy link
Contributor Author

nathan-schmidt-viget commented Dec 19, 2023

@clatwell We may need to move the "Remove WooCommerce page" to another ticket. As it looks like editing/updating the WP navigation block is not straightforward and the documentation is little to none (from what I can find).

It seems like we may be able to it by:

  • Creating a new navigation
  • Adding in our own pages
  • Assigning the header navigation to use our new navigation.

@nathan-schmidt-viget
Copy link
Contributor Author

nathan-schmidt-viget commented Dec 19, 2023

Did some research on the block navigation and seems like there is a ticket open to add a hook to block and navigation - WordPress/gutenberg#54904
But it looks like we can use this halfway/fix for now - WordPress/wordpress-develop#2168 I will try it out and see if it does what we need it to do. That code was never merged...

Copy link
Contributor

@bd-viget bd-viget left a comment

Choose a reason for hiding this comment

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

We're getting into some exciting territory with the Auction Archive!! Let me know if you want to huddle about some of these comments, I'm not sure if it works, but I'd like to explore the possibility of utilizing the archive template.

'post_name' => self::ARCHIVE_SLUG,
);

wp_insert_post( $page );
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating a page here with the same slug as the Archive, have you looked into customizing the Auction Archive Template (Appearance > Editor > Templates > Archive Gb Auction)?

Full disclosure, this is just speculation from me, I've never actually done it yet with FSE, but in theory, I would think we could do it. We can customize the Archive template from the FSE, so we should be able to use a custom template instead of building a new page.

Ping me if you want to chat about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct everything is set up to use Appearance > Editor > Templates > Archive Gb Auction and that is how you edit the Archive page. This is the only way to dynamically add a page to the main navigation as the main nav block pulls from the pages. The downside to this is we need to remove Cart/Account/Shop from the navigation. If we can figure out a way to update the navigation then we don't need this at all.

All of this has been in churn last evening and this morning as I keep finding "solutions" to the navigation, but they don't really fix our bigger problem.

themes/goodbids-nonprofit/inc/patterns.php Outdated Show resolved Hide resolved
Copy link
Contributor

@joshuapease joshuapease left a comment

Choose a reason for hiding this comment

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

Can't say I have much WP knowledge to bring... but it looked good to me

add_action(
'goodbids_init_site',
function () {
$page = array(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use square brackets instead?

[

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it totally should! We are going to change directions a bit for the navigation so I am going to remove this second and see if we need it in the navigation PR.

@nathan-schmidt-viget
Copy link
Contributor Author

@bd-viget I pulled out the navigation part and update the per_page to be set when a new site is init.

Comment on lines -58 to 71
$sample_pattern = [
'name' => 'sample-pattern',
'path' => GOODBIDS_PLUGIN_PATH . 'views/patterns/sample-pattern.php',
'title' => __( 'GoodBids Sample Pattern', 'goodbids' ),
$auction_archive = [
'name' => 'template-archive-auction',
'path' => GOODBIDS_PLUGIN_PATH . 'views/patterns/template-archive-auction.php',
'title' => __( 'Archive Auction', 'goodbids' ),
'categories' => [ 'goodbids' ],
'keywords' => [ 'example', 'template', 'demo' ],
'keywords' => [ 'non-profit', 'starter', 'archive' ],
'inserter' => true,
];

$this->patterns = apply_filters(
'goodbids_block_patterns',
[
$sample_pattern,
$auction_archive,
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moves this from the theme pattern into the plugin pattern

Copy link
Contributor

@bd-viget bd-viget left a comment

Choose a reason for hiding this comment

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

Beautiful! Just a few tiny notes, but should be all set!

return $html;
}

$reward_id = goodbids()->auctions->get_reward_product_id( $post_id );
Copy link
Contributor

Choose a reason for hiding this comment

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

With the update to your Auction page PR, this is a great example of when we would need to pass the ID into the method. 👍


$reward_id = goodbids()->auctions->get_reward_product_id( $post_id );
$product = wc_get_product( $reward_id );
$image_html = $product->get_image();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$image_html = $product->get_image();
return $product->get_image();

I can't remember if the sprintf was doing something before, but if we no longer have anything to pass into the $image_html var, this can go ahead and return here.

add_action(
'goodbids_init_site',
function ( int $site_id ): void {
update_option(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
update_option(
update_option( 'posts_per_page', 9 );

Very minor, but I usually only break things up into multiple lines if there easier to read. In this case, it might easier to keep this on 1 line since it's a pretty simple bit of code.

* @since 1.0.0
* @package GoodBids
*/
?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
?>
?>

Just need to add a line here.

@nathan-schmidt-viget nathan-schmidt-viget merged commit 7ccc537 into main Dec 19, 2023
1 check passed
@nathan-schmidt-viget nathan-schmidt-viget deleted the ns/39-auction-index branch December 19, 2023 21:55
bd-viget added a commit that referenced this pull request Mar 22, 2024
bd-viget added a commit that referenced this pull request Mar 22, 2024
* [#823] Restore Bid Variation due to catastrophic failure

* [#823] Compare with highest value.

* [#823] Make sure Auctioneer gets the correct value.

* [#823] Make sure Bid increments correctly.

* [#103] Forgot to commit this Author code
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.

4 participants