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

Add results check for URL field #682

Closed
wants to merge 1 commit into from
Closed

Add results check for URL field #682

wants to merge 1 commit into from

Conversation

pkevan
Copy link
Contributor

@pkevan pkevan commented Apr 22, 2024

get_term_link can return WP_Error if there is no matching term (https://developer.wordpress.org/reference/functions/get_term_link/) - this additional check protects against this and the warnings produced from attempting to escape this.

if we want to produce a standard URL i.e. home_url this could also be possible, although unsure what the implications are for doing so i.e. indexing the URLs etc

`get_term_link` can return WP_Error if there is no matching term (https://developer.wordpress.org/reference/functions/get_term_link/) - this additional check protects against this and the warnings produced from attempting to escape this.

if we want to produce a standard URL i.e. home_url this could also be possible, although unsure what the implications are for doing so i.e. indexing the URLs etc
@pkevan pkevan added the [Component] Theme The frontend of the pattern directory, pattern lists UI label Apr 22, 2024
@ryelle
Copy link
Contributor

ryelle commented Apr 22, 2024

I think this will always be a "no results" page, so I would rather not output any meta tags — I think @dd32's solution in #681 would be better (checking before we're in the conditional), what do you think of that PR?

@pkevan
Copy link
Contributor Author

pkevan commented Apr 22, 2024

what do you think of that PR?

I should have checked for existing PRs first 😄

@dd32's solution is neater, just not sure on the implication of having them not there - I'd guess it's probably fine, but that was my concern.

@ryelle
Copy link
Contributor

ryelle commented Apr 22, 2024

Thanks, I've merged #681, so I'll close this one.

@ryelle ryelle closed this Apr 22, 2024
@ryelle ryelle deleted the add/term-link-check branch May 2, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Theme The frontend of the pattern directory, pattern lists UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants