refactor: make combo-box use DataProviderController #7044
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
from
3fbad4a to
b3e1d14
Compare
1 task
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
from
e96006a to
223e39f
Compare
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
from
5e2e3bf to
ab03e1e
Compare
|
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
from
ab03e1e to
34ecc64
Compare
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
6 times, most recently
from
1418504 to
f08d006
Compare
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
from
73cc8c8 to
9957d9c
Compare
1 task
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
4 times, most recently
from
bd70e58 to
065f12b
Compare
This was referenced Apr 19, 2024
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
3 times, most recently
from
630a74a to
d8a450b
Compare
1 task
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
from
33d13ba to
8963067
Compare
1 task
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
2 times, most recently
from
fcbea2c to
9e4b5e2
Compare
vursen
changed the title
web-padawan
reviewed
May 24, 2024
| @@ -244,6 +238,8 @@ export const ComboBoxDataProviderMixin = (superClass) => | |||
| this.pageSize = oldPageSize; | |||
| throw new Error('`pageSize` value must be an integer > 0'); | |||
| } | |||
|
|
|||
| this.__dataProviderController.setPageSize(pageSize); | |||
There was a problem hiding this comment.
nit: looks like all the tests pass with clearCache() line below being removed.
But this also happens on main so not related to this PR, can be ignored.
There was a problem hiding this comment.
Good catch, but yeah, I think we could improve this separately. To be safe, I tested it manually and it works in both WC and Flow.
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
4 times, most recently
from
add3e15 to
a022592
Compare
This was referenced May 31, 2024
wip wip remove hasData API from DataProviderController get rid of page-received fix JSDoc add isPlaceholder for backward compatibility reduce diff fix tests remove unnecessary this binding revert some changes, add more tests wip wip update data provider controller tests minimize diff revert incorrect change after rebase minimize diff remove extra checks improve JSDoc fix corner case, add test enhance JSDoc fix tests reassign cache items to trigger filteredItems observer add code comments fix wip revert check for size from setPage fix failing tests remove unnecessary check revert unnecessary changes use logical assignment operator make data provider controller properties and listeners private
vursen
force-pushed
the
refactor/combo-box/data-provider-controller
branch
from
d6318fc to
b4c7985
Compare
vursen
commented
Jun 3, 2024
There was a problem hiding this comment.
note: The new logic in DataProviderController lacks individual test coverage. I plan to add such tests in a separate iteration after this PR is merged.
There was a problem hiding this comment.
Addressed in #7546
DiegoCardoso
approved these changes
Jun 3, 2024
web-padawan
approved these changes
Jun 4, 2024
|
tomivirkki
approved these changes
Jun 14, 2024
|
This ticket/PR has been released with Vaadin 24.5.0.alpha4 and is also targeting the upcoming stable 24.5.0 version. |
1 task
This was referenced Nov 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR refactors the combo-box to use DataProviderController. The refactoring is expected to be non-breaking and fully compatible with the existing ComboBox Flow connector, which is verified by successfully passing all the Flow integration tests.
Depends on
Type of change