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

Update of the search processor for content-based search #16560

Open
wants to merge 2 commits into
base: 2.x
Choose a base branch
from

Conversation

Bournwog
Copy link
Contributor

@Bournwog Bournwog commented Apr 18, 2024

The search processor has been replaced with the one used in MODX 3
Now the search is performed based on the content
The commented-out functionality 'searchActions' has been removed

@Mark-H Mark-H added this to the v2.9.0 milestone Jun 13, 2024
@Mark-H
Copy link
Collaborator

Mark-H commented Jun 13, 2024

From a code review this looks fine for inclusion, just needs to get sufficiently tested, and probably targeting a 2.9 release instead of a 2.8.x?

@Bournwog
Copy link
Contributor Author

I've used it on several sites, it seems ok

@Ruslan-Aleev
Copy link
Collaborator

@Bournwog You have a quick_search_result_max setting in your code, but you didn’t create it; MODX 2 doesn’t have it initially.

@Bournwog
Copy link
Contributor Author

@Ruslan-Aleev yes, I replaced one setting that is hardcoded with another, without setting a variable, if there is no variable, the hardcoded value will be taken

@Ruslan-Aleev
Copy link
Collaborator

@Bournwog I understand how it works, yes. It just seems unfinished =) I would also add a setting.

@Bournwog
Copy link
Contributor Author

@Ruslan-Aleev
add lexicon and settings fix

@Ruslan-Aleev
Copy link
Collaborator

@Bournwog Leave the lexicon only for en, for other languages ​​translations will be pulled from crowdin.

@Bournwog
Copy link
Contributor Author

@Ruslan-Aleev
check it

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.

3 participants