Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[#39] Auction index #103
Changes from 6 commits
e02d989
84dff8f
e6a2c41
10373bd
d34d4de
92a9d4b
03573ac
e55bc45
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 this use square brackets instead?
[
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.
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.
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.
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.
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.
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.
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.
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. 👍
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 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.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.
Moves this from the theme pattern into the plugin pattern
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.
Just need to add a line here.