Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

fix: LEAP-218: Improve performance of search #1601

Merged
merged 26 commits into from
Oct 31, 2023
Merged

fix: LEAP-218: Improve performance of search #1601

merged 26 commits into from
Oct 31, 2023

Conversation

juliosgarbi
Copy link
Contributor

PR fulfills these requirements

  • Commit message(s) and PR title follows the format [fix|feat|ci|chore|doc]: TICKET-ID: Short description of change made ex. fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errors
  • Tests for the changes have been added/updated (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
  • Self-reviewed and ran all changes on a local instance (for bug fixes/features)

Change has impacts in these area(s)

(check all that apply)

  • Product design
  • Frontend

Describe the reason for change

The original search tool from antd tree-select was having problem with performance when it was being used with a long list of choices, as the list was being highlighted, a lot of elements was being changed at the same time creating a big performance problem

What does this fix?

We decided to stop using the original search tool and create our own search tool and attach to tree-select, with our own search tool we have more flexibility and we was able to fix the performance problem removing the highlight when the search matches with the choice item.

What libraries were added/updated?

none

Does this change affect performance?

As the search tool isn't highlighting the matches values, the performance is way better. Now we are using our own search tool to filter the results so we have more flexibility to treat the result as we want

Does this change affect security?

no

What alternative approaches were there?

We have spent some time trying different approaches, but unfortunately the component tree-select from antd doesn't give us a lot of options, we were able to change the way that the search was being done, but it's impossible to deactivate the highlight. Even though that would be possible to improve the performance changing the way that the search works, the items being highlighted would keep being a problem to the performance.

What feature flags were used to cover this change?

fflag_feat_front_lsdv_5451_async_taxonomy_110823_short

Does this PR introduce a breaking change?

(check only one)

  • Yes, and covered entirely by feature flag(s)
  • Yes, and covered partially by feature flag(s)
  • No
  • Not sure (briefly explain the situation below)

What level of testing was included in the change?

(check all that apply)

  • e2e
  • integration
  • unit

@github-actions github-actions bot added the fix label Oct 23, 2023
@juliosgarbi juliosgarbi marked this pull request as draft October 24, 2023 15:56
@juliosgarbi juliosgarbi marked this pull request as ready for review October 24, 2023 15:57
@codecov-commenter
Copy link

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Files Coverage Δ
src/components/NewTaxonomy/NewTaxonomy.tsx 97.67% <100.00%> (+4.12%) ⬆️
src/components/NewTaxonomy/TaxonomySearch.tsx 92.30% <92.30%> (ø)
src/tags/control/Taxonomy/Taxonomy.js 73.22% <64.70%> (+0.53%) ⬆️

📢 Thoughts on this report? Let us know!.

@juliosgarbi
Copy link
Contributor Author

juliosgarbi commented Oct 30, 2023

/git merge master

Successfully pushed new changes:
Merge remote-tracking branch 'origin/master' into fb-leap-218 (d233eb8)

Workflow run

@juliosgarbi juliosgarbi merged commit 52ec54d into master Oct 31, 2023
@juliosgarbi juliosgarbi deleted the fb-leap-218 branch October 31, 2023 12:23
MasherJames pushed a commit to HelloPareto/label-studio-frontend that referenced this pull request Feb 29, 2024
* fix: LEAP-218: Improve performance of search

* add new tests to see how it works between the original ui and our own search

* working with external input search

* implements new search and fix UI

* split taxonomy and taxonomy search

* add spacing line in the end of the file

* add tests

* change the expandedKeys method from its own method to filterDataTree

* fix tests

* add ff and fix some UX problems

* remove useless function

* expand just the mached value

* fix small bugs and add comments

* stop digging if it doesnt have chindrens

* remove preset ff

* check if is leaf

* Update src/components/NewTaxonomy/NewTaxonomy.tsx

Co-authored-by: hlomzik <[email protected]>

* Update src/components/NewTaxonomy/TaxonomySearch.tsx

Co-authored-by: hlomzik <[email protected]>

* Update src/components/NewTaxonomy/TaxonomySearch.tsx

Co-authored-by: hlomzik <[email protected]>

* Update src/components/NewTaxonomy/TaxonomySearch.tsx

Co-authored-by: hlomzik <[email protected]>

* Update src/components/NewTaxonomy/TaxonomySearch.tsx

Co-authored-by: hlomzik <[email protected]>

* Update src/components/NewTaxonomy/NewTaxonomy.tsx

Co-authored-by: hlomzik <[email protected]>

* change changeValue method name to resetValue

* Update src/components/NewTaxonomy/TaxonomySearch.tsx

Co-authored-by: hlomzik <[email protected]>

---------

Co-authored-by: juliosgarbi <[email protected]>
Co-authored-by: hlomzik <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants