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

Add "whole word" search option #1229

Merged
merged 25 commits into from
Oct 28, 2021

Conversation

longrunningprocess
Copy link
Contributor

@longrunningprocess longrunningprocess commented Oct 25, 2021

Description

This feature will allow users to perform "whole word" searches. There are a bunch of ways to test this, please let me know if you'd like me to add any more tests below.

Fixes #1222

Type of Change

  • New feature (non-breaking change which adds functionality)
  • UI change

Screenshots

Provided with test cases below

How Has This Been Tested?

Project used for testing (5491 entries): Alex's project.zip

Functional testing (should work the same in both list and entry view)

  • Searching for sor or SOR should result in 47 matches

image

image

  • Searching for sort or SORT should result in 4 matches

image

image

  • Opening "Options" and checking "Whole word" should narrow the matches down to only 2

image

  • Checking the "Whole word" option should cause the "Options" changed indicator to appear

image

  • After choosing the "Whole word" option, clicking the "Show all" button should clear it
Screen.Recording.2021-10-25.at.3.34.24.PM.mov
  • After choosing the "Whole word" option, refreshing the browser and/or navigating between list and entry views should retain the setting
Screen.Recording.2021-10-25.at.3.41.58.PM.mov
  • Deleting an entry should still retain all search and filtering options

Viewport testing (list view)

  • Filter options should stack on a small viewport, i.e., phones

image

  • Filter options should stack and expand on a medium viewport, i.e., tablets

image

  • Filter options should only expand on a large viewport, i.e., desktops

image

  • Searching by the semantic domain, 2.1.2 should result in 2 matches

image

  • Searching by regex tokens should not result in matches, e.g.,[a-z] should result in 0 matches

image

Viewport testing (entry view)

  • "Advanced" options should always stack when visible, i.e., medium and large viewports

image

Checklist:

  • I have performed a self-review of my own code
  • I have reviewed the title/description of this PR which will be used as the squashed PR commit message
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

Copy link

@alex-larkin alex-larkin left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@palmtreefrb palmtreefrb left a comment

Choose a reason for hiding this comment

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

Everything looks good to me.

Copy link
Collaborator

@rmunn rmunn left a comment

Choose a reason for hiding this comment

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

Good work overall, but a couple issues noted in the comments. In particular the regex one needs to change. I didn't have time to do a thorough review, so it's possible that my comments on scrollDivToId aren't correct and the error-checking isn't necessary.

Once the search is regex-character-safe, I'll be ready to approve this.

Copy link
Collaborator

@rmunn rmunn left a comment

Choose a reason for hiding this comment

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

Looks good now that the regex-escaping code is in. I noticed one bit of end-of-line whitespace (which GitHub's highlighting flagged as I was reviewing the commit, or I would have missed it). That's just cosmetic, though, and even if that doesn't get fixed this PR is good to go.

@longrunningprocess longrunningprocess merged commit 53c2ffc into develop Oct 28, 2021
@longrunningprocess longrunningprocess deleted the feature/1222-whole-word-search-option branch October 28, 2021 13:43
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.

feat: "whole word" search option
4 participants