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

Added search results sort fallback #456

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

litvinovg
Copy link
Contributor

@litvinovg litvinovg commented Apr 2, 2024

VIVO GitHub issue

What does this pull request do?

Added sort fallback option to provide an option to sort on multiple fields.

What's new?

Added vitro-search:hasFallback object property to specify fallback sort options.
Defined raw name field and field sort options in search configuration.
Defined score field for default (relevance) sort option.
Used raw name sort options as fallback for language specific sort options.
Added Added vitro-search:display data property to control display of sort options.

How should this be tested?

  • Reproduce the problem in the issue
  • Test that the pull request fixed the issue
  • Translations should be reviewed by native speakers

Interested parties

@VIVO-project/vivo-committers

@litvinovg litvinovg linked an issue Apr 2, 2024 that may be closed by this pull request
@chenejac chenejac requested a review from milospp April 2, 2024 12:43
Copy link
Contributor

@chenejac chenejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@milospp, @litvinovg I can't reproduce the issue before application of this PR.
I have tried to turn off multilingual UI in the runtime.properties, but it is still sorting well.

Copy link
Contributor

@chenejac chenejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@litvinovg thanks for the PR.

I have reproduced the issue before applying this fix. The PR is fixing the issue. However, there are five options if multilinguality of UI is turned off:

Sort by Relevance
Sort by Title Z-A
Sort by Title A-Z
Sort by Raw title Z-A
Sort by Raw title A-Z

The second and fourth options, as well as third and fifth options are working the same, but the last two options should be hidden (not to confuse the user).

@litvinovg
Copy link
Contributor Author

The second and fourth options, as well as third and fifth options are working the same, but the last two options should be hidden (not to confuse the user).
@chenejac I addressed that in recent commit, please check

@litvinovg litvinovg requested a review from chenejac April 11, 2024 13:26
Copy link
Contributor

@chenejac chenejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@litvinovg thanks for the improvements. It works nice, I have tested the feature with languageFilters and without. Please take a look into my comments.

chenejac
chenejac previously approved these changes Apr 15, 2024
Copy link
Contributor

@bkampe bkampe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Raw" is not used that way in german, I would suggest to simply remove it.

litvinovg and others added 4 commits April 18, 2024 11:26
…h_individuals_vitro.n3

Co-authored-by: Benjamin Kampe <benjamin@fehrmanns.net>
…h_individuals_vitro.n3

Co-authored-by: Benjamin Kampe <benjamin@fehrmanns.net>
…h_individuals_vitro.n3

Co-authored-by: Benjamin Kampe <benjamin@fehrmanns.net>
@litvinovg litvinovg requested a review from chenejac April 22, 2024 10:58
Copy link
Contributor

@chenejac chenejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@litvinovg this looks ok to me.

Copy link
Contributor

@milospp milospp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@litvinovg Thanks for the fix, this is working for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants