-
Notifications
You must be signed in to change notification settings - Fork 1
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
Added Search Terms for Searchable Pages #42
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.
I decided to only have these search terms on the pages, so they reside in one spot. It is flexible enough that we can change this in the future, but I think this should work for most use cases
Approving, but I left a note about using settingsFields
* @param FieldList $fields | ||
* @return void | ||
**/ | ||
public function updateSettingsFields(FieldList $fields) |
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.
This being under settings makes sense for pages, but for other searchable dataobjects, like articles and content layouts, they do not have settings fields.
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.
That's a good point. I did not consider Blog articles...
- I was trying to avoid adding to the CMS fields, simply to not clutter it up any more
- Layouts I am fine without them noy having the option, as they are not searched directly as in they reference a page
- Articles though...they should have the setting somewhere
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.
@tiller1010 Take a look at this again, I updated so the search terms are added to all objects
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 forgot to mention the content layout, blog, etc....need their getIndexQuery functions updated to include search terms. I figure I would do that after this is merged and released
SiteTree objects will have it added to their Settings tab. DataObjects will have it added to their main tab
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.
Nice!
https://werkbotstudios.teamwork.com/app/tasks/35150248
Summary
Testing Steps
Issues/Concerns
Git Flow