Skip to content

Conversation

@tomivirkki
Copy link
Member

@tomivirkki tomivirkki commented May 21, 2021

The current vaadin-grid-array-data-provider-mixin.js, at the end of the day, is just one opinionated dataProvider implementation. Having the array data provider's codebase mixed in into the GridElement class isn't ideal since it adds unnecessary complexity to the component and makes it difficult to add enhancements related to the array data provider logic (see #1563 for more background).

This internal refactor moves the array data provider to an external module (arrayDataProvider.js) which may later be enhanced as a public feature as suggested in #1563.

@tomivirkki tomivirkki marked this pull request as draft May 21, 2021 10:08
@tomivirkki tomivirkki force-pushed the separate-array-data-provider branch from f669e31 to e3f2414 Compare May 21, 2021 11:42
@tomivirkki tomivirkki force-pushed the separate-array-data-provider branch 4 times, most recently from c8153cf to 713f230 Compare June 5, 2021 08:20
@tomivirkki tomivirkki force-pushed the separate-array-data-provider branch from 4cb44f3 to 004f112 Compare June 18, 2021 11:58
@tomivirkki tomivirkki marked this pull request as ready for review June 18, 2021 12:20
@tomivirkki tomivirkki requested a review from vursen June 18, 2021 12:30
Comment on lines -89 to +96
it('should not override custom data provider', () => {
it('should override custom data provider', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed internally that setting grid.items = people is basically a shorthand for setting grid.dataProvider = createArrayDataProvider(people) so setting items should actually override any existing dataProvider

@tomivirkki tomivirkki force-pushed the separate-array-data-provider branch from cc324e3 to 60220b3 Compare June 22, 2021 06:29
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@tomivirkki tomivirkki merged commit 890c526 into master Jun 22, 2021
@tomivirkki tomivirkki deleted the separate-array-data-provider branch June 22, 2021 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants