Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Spring Data API for Grid #7011

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

feat: Spring Data API for Grid #7011

wants to merge 10 commits into from

Conversation

Artur-
Copy link
Member

@Artur- Artur- commented Jan 8, 2025

Provides a Spring Data specific setItems variant based on Pageable.

The method is called setItemsSpring because the signature is the same as existing setItems methods.

Used as e.g.

grid.setItemsPageable(pageable -> productService.list(pageable))

@mstahv
Copy link
Member

mstahv commented Jan 8, 2025

Very good, almost like I would have done it 😁

"Spring" alone in the naming doesn't sound quite right though. I'd at least expand that to SpringData as that is where the Pageable is located in.

Couple of other drafts I tried yesterday in my IDE, but I feel like the best option is missing from the list:

    setItemsPaged
    setItemsPageable
    pageItems

I'd pick this last one if other lazy methods wouldn't already drive people toward setItems, maybe still a good option 🤷‍♂️

@Artur-
Copy link
Member Author

Artur- commented Jan 8, 2025

setItemsPageable is a very technical name but maybe still better then using Spring

@Artur-
Copy link
Member Author

Artur- commented Jan 8, 2025

It intentionally starts with setItems so that it can be auto completed along with all the other variants

Artur- added 8 commits January 9, 2025 08:45
Provides a Spring Data specific setItems variant based on Pageable.

The method is called setItemsSpring because the signature is the same as existing setItems methods.

Used as e.g.

```

grid.setItemsSpring(pageable -> productService.list(pageable))

```
@Artur- Artur- marked this pull request as ready for review January 9, 2025 06:46
@Artur- Artur- requested a review from sissbruecker January 9, 2025 06:47
@Artur-
Copy link
Member Author

Artur- commented Jan 9, 2025

Not sure if we want to add some Spring ITs here or not

@Artur-
Copy link
Member Author

Artur- commented Jan 9, 2025

Well not ITs, unit tests

@Artur-
Copy link
Member Author

Artur- commented Jan 9, 2025

As far as I understand, the current ITs tests that it does not cause issues in non-Spring projects

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.

2 participants