Skip to content

Draft: feat(GlFilteredSearch) Don't treat spaces specially

Mark Florianrequested to merge
1803-filtered-search-spike-do-not-treat-spaces-specially into main

What does this MR do?

This is a spike, and is result of #1803 (closed). It changes GlFilteredSearch to not treat spaces (or any characters in token values in fact) specially. This means all quoting/unquoting logic has been removed, and should address these issues:

Since this is a spike, almost no tests have been updated, so there are a number of broken specs. In theory, most of these should be because expectations have changed, but it's feasible there are new bugs and real breaking changes in here as well.

Before After
simplescreenrecorder-2023-01-09_17.38.56 simplescreenrecorder-2023-01-09_17.33.32

Behaviour (UX) changes

  • 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, or in an existing token's title segment (actually this latter case doesn't work yet). This means that instead of typing fooSpacebarEnter to enter some text and submit the query, you'd now type fooSpacebarEnterEnter (i.e., Enter twice). The first completes the token, the second submits/applies the filter.
  • 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.
  • 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.
  • The token type suggestion list is no longer reduced as you type. Instead, all available token types are listed, and the one that matches is activated. This makes the behaviour identical to token segments with statically defined options (e.g. operators, or "Confidential" in stories). This also means you no longer have to press ↓ to choose a token type after typing (e.g., previously you would type mile↓Enter) to start a Milestone token, whereas now you just type mileEnter
  • When picking token types or statically-defined options, suggestions now match case-insensitively (e.g., "no" now activates the "No" suggestion in "Confidential", whereas before it didn't).
  • Quotes are now treated as regular characters and have no special behaviour. This may impact stored queries (i.e., URL search params), TBD.
  • It's no longer possible to deselect the first/last suggestions by pressing up/down respectively.
  • It's now possible to Tab to the input of the component when there are existing tokens. Previously, the mouse had to be used to click in the blank space between the existing tokens and the clear button. (See also #1493 (moved).)
  • For statically defined suggestions without an explicit default, the first token is no longer automatically activated. In practice this probably affects operators the most, but even then they usually define a default, so there should be no noticeable change. TBD.

Breaking changes

  • For tokens with statically-defined options, picking one of those options now uses that option's value instead of its title.
  • ???

To do

  • Make tokens like Weight (which accept free text input) display the Search for this text suggestion. This also applies to tokens which allow suggested values and arbitrary input (if there are such uses cases?).
  • Link into GitLab, see/document what breaks. See gitlab!108481 (closed) and #1803 (comment 1232461845).
  • Fix all the specs!

Bugs introduced

  • Occasionally a blank token with no type gets added. Not sure how/why. blank_token
  • A blank operator token is added if you choose a token type, then press Escape when choosing an operator.
  • A blank operator token is added if you choose a token type, then press RightArrow before typing or choosing an operator.
  • You can't Tab past a GlFilteredSearch, as you end up in a loop within the component. This is a serious a11y regression, but definitely fixable outside the scope of this spike.

Bugs found fixed

See #2114 (moved) for the full list, but I decided to fix the following in this MR, either because it was easy to do, or because it was important for this feature change.

  • Suggestions not being highlighted unless text is entered with the correct casing
  • Inconsistent quote handling
  • An input is not always rendered, so you can’t always start editing the search input by keyboard only
  • The options[].title being used as the token value instead of options[].value
  • A runtime error if you type a quoted string followed by a space
  • A runtime error if you choose the Author token, press up arrow to deselect the operator suggestion, then press Escape
  • A runtime error when you choose a token type, then press RightArrow before typing or choosing an operator.
  • Ambiguously-typed check in isLastToken
  • It's possible to deselect the first/last suggestions by pressing up/down respectively
  • Matching suggestions aren't activated, e.g., type "auth", and only the Author token suggestion is available. Why not activate it?
  • Inconsistent behaviour of suggestions for a term and a segment. In a term, typing reduces the list of visible options. For a segment, it only maybe activates one of them. This is because suggestion display logic lives in both Term and Segment components, and are implemented differently.
  • When show-friendly-text is enabled, typing any key except ! or = chooses the first operator, and then that key is placed in the next segment.
  • An unused method

Misc notes

It's odd that a token definition's options has a different shape/interface to the availableTokens prop:

type Option = {
  icon: string
  title: string
  value: string
}
type AvailableToken = {
  icon: string
  title: string
  type: string
}

Should these be of the same "type"? I suppose not. But maybe the options property should be renamed suggestions, so it's more obviously related to the suggestions slot?

Closes #1803 (closed)

Edited by Mark Florian

Merge request reports