Skip to content

feat(GlFilteredSearch): Terms as tokens

Mark Florian requested to merge 2159-filtered-search-stop-treating-spaces-specially into main

What does this MR do?

This adds a new opt-in prop terms-as-tokens which changes the rendering/interaction logic of free text search terms.

This is the third and final of a series of MRs, aiming to address #2159 (closed).

  1. GlFilteredSearch: Miscellaneous fixes and impro... (!3362 - merged)
  2. GlFilteredSearch: Improve option interactions (!3368 - merged)
  3. This MR 👈

This also addresses these issues:

Existing uses of this component should be unaffected. Opting into this changes rendering and interactions in the following ways:

  • This is the big one: free text is now entered via a special "Search for this text" suggestion that appears once you start typing in a new token input. This means that instead of typing f o o Space b a r Enter to enter some text and submit the query, you'd now type f o o Space b a r Enter Enter (i.e., Enter twice). The first completes the token, the second submits/applies the filter. This seems like a reasonable trade-off for better value handling/editing.
  • Free text tokens are now rendered as tokens, with working close/dismiss buttons. Clicking on the tokens activates them as normal, and all the text can be edited normally.
  • A space character no longer "completes" a token. This means searching for a token value with a space on it no longer requires the undiscoverable quoting "trick", and it just works in the expected way.
  • It's no longer possible to deselect the first/last suggestions by pressing up/down respectively. This is so that the Search for this text option cannot be deselected, which would lead to weird/undefined behaviour.
  • Quotes are now treated as regular characters and have no special behaviour. This may impact stored queries (i.e., URL search params), TBD.

Screenshots

Before After
storyshots-spec-js-image-storyshots-base-filtered-search-default-1-snap storyshots-spec-js-image-storyshots-base-filtered-search-with-terms-as-tokens-1-snap
simplescreenrecorder-2023-01-09_17.38.56
NOTE: This video is from an earlier iteration, and so is lacking some improvements made in !3362 (merged) and !3368 (merged).		 simplescreenrecorder-2023-01-09_17.33.32
NOTE: This video is from an earlier iteration, and so isn't quite representative of what's in this MR. The only difference I think is visible here is that the Search placeholder is no longer displayed when there are existing tokens and the input is inactive (i.e., it is now how it is on current main). See the screenshot above for an accurate representation of this MR.

Notes for reviewing this MR

  • It's marked as draft, since it's based on and targeting the filtered-search-misc-fixes branch (i.e., !3368 (merged)). When that merges, this should automatically retarget main. The draft status can then be removed.
  • You might find some of it easier to review by unchecking Show whitespace changes, particularly for the tests, as there are some indentation-only changes.
  • You might find it easier to review it commit-by-commit. Or not!
  • This MR is split into two commits. The first implements the new terms-as-tokens prop, and the second adds tests for the new behaviour. This is done for two reasons:
    • The first commit, without (major) test changes, demonstrates that no existing behaviour is changed/broken by the new prop.
    • This is quite a large MR, so splitting out the implementation from the tests might make reviewing it easier.

Does this MR meet the acceptance criteria?

Conformity

  • Code review guidelines.
  • GitLab UI's contributing guidlines.
  • If it changes a Pajamas-compliant component's look & feel, the MR has been reviewed by a UX designer.
  • If it changes GitLab UI's documentation guidelines, the MR has been reviewed by a Technical Writer.
  • If the MR changes a component's API, integration MR(s) have been opened in the following projects to ensure that the @gitlab/ui package can be upgraded quickly after the changes are released:
  • Added the ~"component:*" label(s) if applicable.

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • Security reports checked/validated by a reviewer from the AppSec team

Accessibility

If this MR adds or modifies a component, take a few moments to review the following:

  • All actions and functionality can be done with a keyboard.
  • Links, buttons, and controls have a visible focus state.
  • All content is presented in text or with a text equivalent. For example, alt text for SVG, or aria-label for icons that have meaning or perform actions.
  • Changes in a component’s state are announced by a screen reader. For example, changing aria-expanded="false" to aria-expanded="true" when an accordion is expanded.
  • Color combinations have sufficient contrast.

Closes #2159 (closed)

Edited by Mark Florian

Merge request reports