-
Notifications
You must be signed in to change notification settings - Fork 16
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
Button to load more results has incorrect and misleading text #36
Comments
This behavior has changed but there are still issues with it. To update on the original report, here's the current behavior:
Problematic test caseTake a query that has a PREFIX wdt: <http://www.wikidata.org/prop/direct/>
PREFIX wd: <http://www.wikidata.org/entity/>
SELECT DISTINCT ?item ?type WHERE {
?item wdt:P279 wd:Q11002 .
}
LIMIT 1000 After executing this query, the table shows all the 7 results. However:
LogicThe UI code has this logic: let limitMatch = result.query.match(/\bLIMIT\s+(\d+)\s*$/);
if (limitMatch) { resultSize = parseInt(limitMatch[1]); } (The regex causes confusion e.g. if the Before these lines, The current UI logic clearly doesn't work when there's a The inputs we have:
Invariant: Invariant when Invariant when there's a
The current condition to show the second button: if (nofRows < parseInt(resultSize)) { I think the condition should instead be |
If you execute the above query, the button on top of the results table says
Limited to 40 results. Show all 3,018,709 results.
. If you click on it, the results table does NOT show 3M results, but shows 1000 results. This is confusing.The button text should tell me what will really happen if I click on it. In this case, it should recognize the
LIMIT
in the query and sayShow 1000 results
.It might even make sense to implement a two-step process: The table shows 40 results. The button says
Show 1000 results
. I click on the button to show the 1000 results I specified in the query. Now the button saysShow all 3,018,709 results
. If I click on it, all 3M rows are shown.The text was updated successfully, but these errors were encountered: