Skip to content
Verified Commit 88dccde9 authored by Mark Florian's avatar Mark Florian
Browse files

fix(GlFilteredSearch): Generate stable token keys

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:

1. 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.
2. 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 gitlab-org/gitlab-ui#1761.
parent d5f2533e
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment