Skip to content

Conversation

HichamELBSI
Copy link
Collaborator

What does it do?

Add optional virtualized list for combobox (implemented to the timepicker).

Why is it needed?

To improve the TimePicker (and Combobox) performances on long list.

How to test it?

  • Set step = 1 to a TimePicker and try to use it.

Related issue(s)/PR(s)

Let us know if this is related to any issue/pull request

Before fix: https://www.loom.com/share/07fc90f6b02543b48e18a3f036150c70

After fix: https://www.loom.com/share/f740d3fd31424e6285ea05517bebb342

@HichamELBSI HichamELBSI requested review from Adzouz and butcherZ October 9, 2025 15:20
Copy link

changeset-bot bot commented Oct 9, 2025

🦋 Changeset detected

Latest commit: 573c5fa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@strapi/design-system Major
@strapi/ui-primitives Major
@strapi/icons Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Oct 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
design-system Ready Ready Preview Comment Oct 17, 2025 7:55pm

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

Copy link
Contributor

@Adzouz Adzouz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and it really improves performance, well done 👏
Just a few questions / comments!

onChange,
onClear,
onCreateOption,
virtualized = false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question here: I get that we want to keep the default behaviour as much as possible but wouldn't it be also possible to turn on the virtualization automatically if the number of items exceed a limit (for example for a dynamic list we don't know how many elements it could have)?

@@ -0,0 +1,6 @@
---
'@strapi/design-system': major
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's not really breaking anything, is it really necessary to bump to a major?

'@strapi/ui-primitives': major
---

Add virtualization as an option to combobox list
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decided to also stick to conventional commits for changesets messages format :)

@remidej remidej removed their request for review October 14, 2025 12:13
@HichamELBSI HichamELBSI force-pushed the fix/virtualized-combobox branch from d83aa0f to 3f031d4 Compare October 15, 2025 08:12
@HichamELBSI HichamELBSI removed the request for review from markkaylor October 15, 2025 11:48
@HichamELBSI
Copy link
Collaborator Author

HichamELBSI commented Oct 15, 2025

There are some issue with the combobox input events (change, clear, blur) because of the virtualization. I'll post pone the fix on the combobox performance.

@HichamELBSI HichamELBSI added the flag: don't merge This PR should not be merged at the moment label Oct 15, 2025
@HichamELBSI HichamELBSI force-pushed the fix/virtualized-combobox branch from 3f031d4 to 75e38fd Compare October 17, 2025 19:49
@HichamELBSI HichamELBSI force-pushed the fix/virtualized-combobox branch from 75e38fd to 573c5fa Compare October 17, 2025 19:52
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@HichamELBSI
Copy link
Collaborator Author

The issue has been fixed by disabling the virtualization as soon as the list is being filtered. It solves most of the issues.

@HichamELBSI HichamELBSI removed the flag: don't merge This PR should not be merged at the moment label Oct 17, 2025
@HichamELBSI HichamELBSI requested a review from Adzouz October 17, 2025 19:53
@HichamELBSI HichamELBSI added this to the v2.0.0 milestone Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hacktoberfest-accepted pr: fix This PR is fixing a bug source: design-system relates to design-system package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants