Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
On websites with a large amount of content, the linkit widget will show all the results available for a certain search query. This slows the widget down considerably.
Proposed resolution
Add a setting to the EntityMatcher to add a limit to the query executed.
Remaining tasks
Supply a patch + tests
User interface changes
Added form element for entity matchers for entering a 'limit'.
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | linkit-add-limit-2980649-25.patch | 3.31 KB | jecunningham2281 |
| #18 | linkit-add_limit-2980649-18.patch | 4.34 KB | nuez |
| #15 | linkit-add_limit-2980649-15.patch | 23.07 KB | nuez |
| #15 | interdiff-13-15.txt | 14.82 KB | nuez |
| #13 | interdiff-8-13.txt | 5.55 KB | nuez |
Comments
Comment #2
nuezHere's the patch
Comment #3
nuezComment #5
nuezComment #6
nuezComment #8
nuezTests are failing because of the schema definition of the 'limit', which cannot be 'integer', because we have a default value of '', which is a string. We could also choose to set a default value of 0, in that case it could continue to be an integer.
But i think this is more elegant.
Comment #9
nuezComment #10
rbayliss CreditAttribution: rbayliss at Last Call Media for Commonwealth of Massachusetts commentedThanks for the patch! It works well, but I can't see a reason for having the default value be a string. Care to elaborate on why it's more elegant?
Comment #11
nuezOf course, I'll try to explain:
We basically have two options:
1. Set the default value of
limitto 0 and the data type to 'integer'.2. Set the default value of
limitto '' and the data type to 'string'.Personally I would prefer, that if you don't want to use the limit, you simply don't fill in the field. In that case, you bascally fill in an empty string (in other words leave the form element empty). The default value for the
limitwould be defined as an empty string in::defaultConfiguration()If we do this however, and at the same time we set the schema data type of 'limit' to 'integer', our test breaks because of the strict schema checking: an empty string is not an integer.
The alternative would be to say that the default value of
limitis 0 (type integer), in which case no limit would be applied. That might be more 'correct' from a programming point of view, because we could set the data type of the limit to integer without it breaking the tests. However from an UI point of view, it's less intuitive. Simply leaving the form element empty would be more intuitive IMO.I'm not sure if there is another solution to this, (setting the value to 0 when empty in the validation callback, or not setting the default value of the limit property)
Please let me know if you have any suggestions.
Comment #12
rbayliss CreditAttribution: rbayliss at Last Call Media for Commonwealth of Massachusetts commentedSure - I understand what you're saying. For the UI, how about if we use a select list with predefined options?
- Unlimited
- 20
- 50
- 100
- 200
That seems like decent UX and we can still key the select options on a numeric value (0 = Unlimited).
Comment #13
nuezGood idea.
Comment #15
nuezComment #16
nuezComment #17
Berdir+1 to the feature, but looks like the patch also contains some patch files?
Not sure about backwards compatibility but I would suggest to set the default to a fixed limit and not unlimited.
Comment #18
nuezUps.
Would backwards compatibility not be a reason to not set a default value?
I agree however that unlimited results is not desired in most cases.
If any limit, I would set one high enough that current sites using this module wouldn't notice the difference: say 100.
Comment #19
BerdirYes exactly, was thinking the same, I doubt anyone will complain if they suddenly get "only" 100 results...
Comment #20
anonJust to be clear, the limit is for each matcher. With that said, if you have 2 entity matchers, with a limit of 100 in both, you could get a max of 200 results.
Is this is correct behavior?
Comment #21
BerdirDefine "correct" :) I suppose a combined max result would in some ways be better, but it would also be far more complex to implement, as it would require making the limit part of the API and having the configuration stored in the profile. So changes to the API/Interface would be necessary.
But that has downsides too, because it means that you will never get any results from secondary matchers if the first one has a lot results.
IMHO, this is acceptable, if you have multiple matchers, you can set them all to a lower value like 20.
Comment #23
anonThis is okey by me. Just wanted to make it clear. Thanks for patch.
Comment #25
jecunningham2281 CreditAttribution: jecunningham2281 commentedRe-rolling patch for linkit version 4.3