Skip to content

GlFilteredSearch: Miscellaneous fixes and improvements

Mark Florianrequested to merge
filtered-search-minor-fixes into main

What does this MR do?

This fixes various minor issues with GlFilteredSearch.

Part of #2159 (closed).

docs(GlFilteredSearch): Fix token options properties

The text property as far as I can tell has never existed. Instead, there's a title property that has the same function.

fix(GlFilteredSearch): Improve activeTokenIdx check in isLastToken

This negation is truthy when activeTokenIdx is null (there's no active token) and when it's 0 (the first token is active).

This means that the last token is told it's not the last token if anything other than the first token is active. We don't want to treat the first token specially like this, so instead we check just for null.

chore(GlFilteredSearch): Simplify template

The getTokenClassList method was previously unused, and so the logic was actually duplicated.

fix(GlFilteredSearch): Remove conditional class

The class had no effect, since the .gl-filtered-search-token-segment class already specifies width: 100%.

fix(GlFilteredSearch): Improve keyboard accessibility

This ensures that the token option list is shown/hidden correctly when the input is tabbed to/away from.

chore(GlFilteredSearch): Render milestone token dropdown divider

refactor(GlFilteredSearch): Use better variable name

It's an option we (may) have found, so let's call it that.

fix(GlFilteredSearch): Avoid rare scroll method error

Under certain rare circumstances, the ref or its element does not exist when this callback runs.

fix(GlFilteredSearch): Improve handling of some falsey values

This allows false, 0 and '' to be handled better as values in options. Previously, an option with one of these values could never get automatically selected when typing their title in a segment.

An example where this can be seen is the Confidential token in Roadmaps: https://gitlab.com/gitlab-org/gitlab/-/blob/2dcb4f37a26ffff6b2cebc6b1f55f1294b1b545d/ee/app/assets/javascripts/roadmap/mixins/filtered_search_mixin.js#L179-180.

The null and undefined (and NaN) values are still ignored.

refactor(GlCollapsibleListbox): Reuse computed property

test(GlFilteredSearch): Reuse helper

Screenshots

This is the only real user-facing change which isn't too tricky to demonstrate:

Before After
simplescreenrecorder-2023-03-30_18.42.35 simplescreenrecorder-2023-03-30_18.39.46

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.
Edited by Mark Florian

Merge request reports