-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
[FEAT] Allow for queries/filters on collections when generating the sitemap #109
base: master
Are you sure you want to change the base?
Conversation
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.
A quick note about i18n
this is amazing. just want to thank @Kyle772 for this feature, i had a hard time to extend the plugin to do the same thing <3 |
I've updated the settings schema a bit. The PR was initially propsed with a settings object that might look like this:
I've updated the PR so that the filters will be saved separately from the other pattern settings. Which could lead to a settings object that might look like this:
|
dispatch(onChangeContentTypes(uid, null, ['filters', conditionCount, 'operator'], '')); | ||
dispatch(onChangeContentTypes(uid, null, ['filters', conditionCount, 'value'], '')); | ||
setConditionCount(conditionCount - 1); | ||
}; |
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.
The handleRemoveCondition
function always removes the last filter. It would be neat if we can specify the filter we want to remove. Giving the end user more control on the filters interface.
}; | ||
return ( | ||
<div> | ||
{conditionCount === 0 ? ( |
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 think this condition can be optimized.
The only thing that should/shouldn't show for this ternary is the list of conditions. Though this ternary is used to render the entire component leaving duplicate code below (e.g. the add/remove buttons).
If we use the ternary just for the conditions list this would be resolved.
Hi @Kyle772 Have you been able to take a look at the feedback I've posted on the PR? |
Wow I'm so sorry I don't ever check my github inbox lol. I'll try to address these changes sometime over the next couple of weeks. Sort of slammed at the moment but would love for this to get merged as I use my own fork on my sites |
What does it do?
This implements conditions into the collections queries.
Below is a SS of the UI change.
Why is it needed?
The general idea here is to make the sitemaps more flexible. In my case, I wanted to filter out anything that was removed from the website when an entry was marked as deleted (but not unpublished so we can retain data where necessary; ie invoicing).
How to test it?
Nothing major was added so you should be able to run this as you normally would
Related issue(s)/PR(s)
Let us know if this is related to any issue/pull request
I think this only impacts the below issue but there may be some others.
#99