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

Dropdown Lists of OneToMany relation shows only the first 20 items #6339

Closed
1 task done
stieler-it opened this issue Sep 12, 2017 · 19 comments
Closed
1 task done

Dropdown Lists of OneToMany relation shows only the first 20 items #6339

stieler-it opened this issue Sep 12, 2017 · 19 comments
Milestone

Comments

@stieler-it
Copy link
Contributor

stieler-it commented Sep 12, 2017

Overview of the issue

Dropdown lists for OneToMany relations show only the first 20 options. (Probably this happens due to active pagination / endless scrolling).

Motivation for or Use Case

It is not possible to select every available option.

Reproduce the error
  • Define two entities: Employee and Department. Employee has a OneToMany relationship to Department.
  • Create 21 departments
  • While creating an employee, open the department list

Expected behaviour: The list shows all 21 departments
Actual behaviour: The list shows only the first 20 departments

Related issues

n/a

Suggest a Fix

Where the 20 comes from:

org.springframework.data.web.PageableHandlerMethodArgumentResolver defines DEFAULT_PAGE_REQUEST = new PageRequest(0, 20); which is used when

  1. The default is not overridden in the Resource
  2. The REST client does not provide pagination options

What we could do:

  1. Generate a PageableDefault Annotation into the getAllDepartments() method in the Resource
  2. Provide paging options to this.departmentService.query() in employee-dialog.component.ts, e.g. this.departmentService.query({size: 2000, sort: ['name,asc']})

The page size seems to be limited to 2000 by PageableHandlerMethodArgumentResolver. However, I don't think showing so many entries in a dropdown anyway.

JHipster Version(s)

4.6.2

JHipster configuration

.yo-rc.json

{
  "generator-jhipster": {
    "promptValues": {
      "packageName": "XXX",
      "nativeLanguage": "de"
    },
    "jhipsterVersion": "4.6.2",
    "baseName": "babsapp",
    "packageName": "XXX",
    "packageFolder": "XXX",
    "serverPort": "8080",
    "authenticationType": "oauth2",
    "hibernateCache": "no",
    "clusteredHttpSession": false,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "h2Memory",
    "prodDatabaseType": "oracle",
    "searchEngine": "elasticsearch",
    "messageBroker": false,
    "serviceDiscoveryType": false,
    "buildTool": "maven",
    "enableSocialSignIn": false,
    "clientFramework": "angularX",
    "useSass": true,
    "clientPackageManager": "yarn",
    "applicationType": "monolith",
    "testFrameworks": [
      "protractor"
    ],
    "jhiPrefix": "jhi",
    "enableTranslation": true,
    "nativeLanguage": "de",
    "languages": [
      "de"
    ]
  }
}
Entity configuration(s) entityName.json files generated in the .jhipster directory

department.json

{
    "pagination": "infinite-scroll",
   ...
}
Browsers and Operating System

Chrome, Windows 7, Java 8

  • Checking this box is mandatory (this is just to show you read everything)
@jdubois
Copy link
Member

jdubois commented Sep 12, 2017

In fact that's all normal:

  • Yes if you select pagination, then you are limited to 20 by default (otherwise don't choose pagination)
  • And yes if you want to increase the pagination level, you can do it first on the client side (normal REST API), and then you need to tune Spring Data (also normal API usage here, and indeed this looks like a bad idea to increase this)

So what you write is totally the idea, so as far as I'm concerned there's no bug here. But probably that needs to be better documented?

@stieler-it
Copy link
Contributor Author

stieler-it commented Sep 13, 2017

If I select pagination, I expect my list views to be paginated of course. The user can either switch pages or scroll, depending on the pagination mode.
However, I don't want my select lists to be paginated because they don't support paging! The user is not able to switch pages and thus cannot reach all entities here.
In my eyes this is a major bug as the generated application shows wrong / incomplete selection lists. It is also not a corner case, as it happens already with 21 entities.

@gzsombor
Copy link
Member

It's a long known limitation. Using dropdowns for selecting from more than a couple of possibilities is a UX problem. This should handled with autocomplete/type ahead components.
I think, this can be implemented by JHipster using for example with https://ng-bootstrap.github.io/#/components/typeahead/examples and the recently created filtering.
If you have time, feel free to start working on it, and contribute back 😉

@jdubois
Copy link
Member

jdubois commented Sep 13, 2017

We could increase the size of the list on the client size, that's easy. Then, if we display drop-down lists that are bigger than 20 it's indeed a UX problem, and there will still be a limit at some point, so this isn't really solving anything.

@deepu105
Copy link
Member

I agree to the suggestion by @gzsombor but its tricky as filtering is not enabled by default. Another option would be to use typeahead component instead of select for entities that have pagination enabled. This would require us to generate an extra query for the typeahead search on the field used to display the relationship. Its not difficult to do but takes time so @stieler-it if you are up for it you can try to do a PR

@jdubois
Copy link
Member

jdubois commented Sep 16, 2017

For the type ahead: for me the generated front end is already too complex. We need to have something simple that people can easily understand and modify.
So I'm against it.
I think we should modify the Angular service to explicitly fetch 20 entities: it's very simple to do, and would show people how to modify this easily.

@gzsombor
Copy link
Member

We can easily generate separate typeahead component for all the entities, which can be re-used everywhere. This in fact simplify the dialog components (as the entity loading could be removed from the dialog component), so we can write in the html:
<jhi-author-autocomplete [(ngModel)]="book.author"></jhi-author-autocomplete>

If either elasticsearch or entity filtering is enabled, then this component could be a true autocomplete, if not, it would just load the first 20 entity, and show a dropdown - as currently we are doing.

@stieler-it
Copy link
Contributor Author

Elasticsearch and entity filtering are global settings, right? I think it depends on the actual entity if a typeahead component is better as it assumes that the user knows a part of the wanted entry, which is not always the case.

So maybe a

<jhi-author-select [(ngModel)]="book.author" type="typeahead"></jhi-author-select>

would be more flexible?

By the way, in my opinion showing more than 20 items in a drop down list is not automatically an UX problem and if so, I would still prefer showing a big complete list over showing an incomplete list.

In my opinion we should first fix/change the code to return all/many items. A developer would still be able to replace the select box with something more sophisticated if required.

I like the idea of @gzsombor but I would see it as a second step and as an improvement, not a bugfix.

@jdubois
Copy link
Member

jdubois commented Sep 19, 2017

The other issue I have with the typeahead component, is which one should we use? We always had that philosophy of doing something opinionated, but just enough to do the job - then people add the libraries they like for everything else, and that includes the typeahead for me.
I know it will end up with people complaining we didn't select the right typeahead, or that we shouldn't have coded our own component, etc etc. Which means more maintenance and lots of discussion.

@stieler-it
Copy link
Contributor Author

Second that. Although we could probably just use ng bootstrap typeahead which is already included in JHipster before adding another dependency.

@jdubois
Copy link
Member

jdubois commented Sep 19, 2017

@stieler-it you are right -> if that's included in NG Bootstrap then that's good, as we are officially based on Bootstrap. I just thought it wasn't ready yet. Then we could go this way, OK.

@stieler-it
Copy link
Contributor Author

Still I don't think typeahead would be a good solution for all relations. In our case we would be happy to have a regular select box (showing 20+ entries, though) and that we could replace it by something more sophisticated as needed. In fact we have enabled pagination for all our entities by default and switching to typeahead would mean to always hide the available options to the user.

So in my opinion to follow JHipster's priciniples of generating simple but customizable code, an easy solution to this issue would just be to load "all" entries by default.

If you insist on a typeahead being part of the solution, I would go for @gzsombor 's approach, extended by a type attribute so the user could easily switch between both implementations.

We could have two implementations:

<jhi-author-select [(ngModel)]="book.author" selectType="typeahead"></jhi-author-select>

and

<jhi-author-select [(ngModel)]="book.author" selectType="dropdown"></jhi-author-select>

I am not sure which one should be the default?

@jdubois
Copy link
Member

jdubois commented Oct 2, 2017

Sorry this has been stuck for nearly 2 weeks.... So what do we do? In the end I think @stieler-it and me agree on having a simple drop-down, but I see more votes for a type-ahead....
-> another solution is to add better documentation: how to have bigger drop-downs (using the "size" parameter), and how to use a type-ahead. There are so many different cases and opinions here, I would just like to keep generating something simple, and then give advices on how to change that.

@stieler-it
Copy link
Contributor Author

I just implemented a workaround in our application. In every affected Angular service, I added:

    query(req?: any): Observable<ResponseWrapper> {
        if (!req) {
            req = {
                page: 0,
                size: MAX_PAGE_SIZE,
                sort: ['name,asc', 'id']
            };
        }

@jdubois
Copy link
Member

jdubois commented Jan 18, 2018

OK this has been opened for too long, so let's move forward:

  • The type-ahead is too complex for many people, I don't want to have this by default
  • The comment above by @stieler-it is exactly what I have wanted from the very start (when I coded this with AngularJS 1), so that's the perfect solution

-> I'm closing this as we have a good solution here, and as this has been stuck for too long. It's also a "normal" solution with Angular, so nothing surprising here.

@jdubois jdubois closed this as completed Jan 18, 2018
@stieler-it
Copy link
Contributor Author

Hi @jdubois - if you like my solution shouldn't we create a PR then instead of closing this issue?

@agoncal
Copy link
Contributor

agoncal commented Jan 19, 2018

@stieler-it BTW there was also an issue about Pageable, and one still going on about Auto-complete fields for the entities (which looks like it will make it for JHipster 5)

@jdubois
Copy link
Member

jdubois commented Jan 21, 2018

@stieler-it it's just that for most people would use the default values, so that code wouldn't be useful for them - for me that's interesting as a tip or as documentation, and that's not even specific to JHipster

@jdubois
Copy link
Member

jdubois commented Jan 21, 2018

But indeed with Spring Boot 2 there will be a global, standard configuration on the server-side, and that will probably be a better solution for everyone

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

No branches or pull requests

5 participants