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

feat(search): improved search snippet FE logic #6109

Merged

Conversation

gabe-lyons
Copy link
Contributor

  1. We used to only show search snippets for datasets if there was no description or title match. Now, we will show them regardless.

  2. We used to just pluck out the first search snippet that came back & was relevant. Now, we do a few passes to try and find the best search snippet based on your query.

Added some unit tests & comments since we are encoding some business logic in this utility.

CleanShot 2022-10-03 at 16 16 23@2x

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added the product PR or Issue related to the DataHub UI/UX label Oct 3, 2022
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Unit Test Results (build & test)

584 tests   580 ✔️  12m 55s ⏱️
143 suites      4 💤
143 files        0

Results for commit 1b9297b.

♻️ This comment has been updated with latest results.

}

return matchedFields.find((field) => FIELDS_TO_HIGHLIGHT.has(field.name));
return fromQueryGetBestMatch(
matchedFields.filter((field) => FIELDS_TO_HIGHLIGHT.has(field.name)),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it would be cleaner to give the first arg to this function a descriptive name


function fromQueryGetBestMatch(selectedMatchedFields: MatchedField[], rawQuery: string) {
const query = normalize(rawQuery);
// first lets see if there's an exact match between a field value and the query
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for the comments!

@gabe-lyons gabe-lyons merged commit 1325b8a into datahub-project:master Oct 4, 2022
david-leifker pushed a commit to david-leifker/datahub that referenced this pull request Oct 6, 2022
* starting improvements on search snippet

* flesh out ranking logic

* adding tests

* responding to comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants