add config option for typeahead limit #3363

Merged
merged 4 commits into from Apr 24, 2016

Projects

None yet

4 participants

@wiad
Contributor
wiad commented Apr 12, 2016

Added config option:
$config['typeahead_limit']

to set limit for typeahead results. Default is set to 5 (as default was before).

Fixes librenms/librenms#3362

@wiad wiad add config option for typeahead limit
362bcd4
@laf
Member
laf commented Apr 19, 2016

Whilst the config name isn't right, this PR introduced a config option you can use: #2957

If not, then this will still need to be updated to be a db config option.

@laf laf added the WebUI label Apr 19, 2016
@wiad
Contributor
wiad commented Apr 19, 2016

Ah, I missed that PR. It is two different things however, though related so both need to be adjusted. The one in librenms/librenms#2957 controls how many results are fetched from the db, and the typeahead config in this PR controls how many of these results that are actually shown.

So even if you set the webui setting for max search limit to '20', you're still only going to see 5 typeahead results without this setting.

My suggestion would be that the
['webui']['global_search_result_limit']

configuration option is used to set the typeahead limit as well (thereby scrapping the "config['typeahead_limit']" option suggested at first in this PR). That should be the desired result, or am I missing some usecase?

@laf
Member
laf commented Apr 19, 2016

Unless I'm missing something then surely saying I want to see 20 db results translates to I want to see 20 search results so we can re-use the config option?

@paulgear paulgear added the Conflict label Apr 20, 2016
Adam Winberg added some commits Apr 21, 2016
Adam Winberg use existings webui option (global_search_result_limit) to set typeah…
…ead limit instead of introducing new config option
341869e
Adam Winberg Merge branch 'master' of https://github.com:/wiad/librenms into issue…
…-3362
9c58f70
Adam Winberg forgot setting typeahead_limit option for dash
8ef8c33
@laf laf removed the Conflict label Apr 24, 2016
@laf laf merged commit 1e309ee into librenms:master Apr 24, 2016

3 checks passed

Auto-Deploy Build finished. No test results found.
Details
Scrutinizer 4 new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wiad wiad deleted the wiad:issue-3362 branch Apr 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment