fix(GlFilteredSearch): Generate stable token keys
What does this MR do?
This fixes incorrect behaviour when destroying a token which has another token of the same type after it. The incorrect behaviour is to activate the next token; the correct behaviour is not to activate it.
There were two underlying causes to this incorrect behaviour:
-
The
key
used for tokens was insufficiently unique. It was just${token.type}-${tokenIndex}
. This meant that if you had the following:UI: Label=Bug Label=Feature keys: label-0 label-1
Then destroying the Bug label token would lead to this:
UI: Label=Feature keys: label-0
Meaning that the component instance and DOM for the first token was reused for the second after the first is destroyed.
-
The mousedown event that causes the token to be destroyed bubbles up to another handler that causes the token to be activated. Because of the DOM reuse mentioned above, the second token gets activated.
Both of these need to be fixed for the behaviour to be correct. The latter is trivially fixed by stopping the event from bubbling up. The former is a bit more involved.
The general strategy is to attach a unique id
property to each token,
that is preserved for the lifetime of that token, which is used as the
key
for the token's component.
Finally, a regression test for this was added in both Jest and Cypress.
The former passes with just the event bubbling fix, perhaps due jsdom
being used instead of a real browser. The Cypress test correctly
requires both fixes to pass.
Addresses #1761 (closed).
Screenshots
Before | After |
---|---|
before_gl-f-s Incorrect behaviours:
|
after_gl-f-s |
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:-
GitLab: GitLab UI integration branch for fix-filtered-s... (gitlab!83868 - closed) - [-] CustomersDot: mr_url
- [-] Status Page: mr_url
-
-
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"
toaria-expanded="true"
when an accordion is expanded. - [-] Color combinations have sufficient contrast.