Skip to content

Commit

Permalink
Improve UX for "in order" option, fix #335
Browse files Browse the repository at this point in the history
Squashed commit of the following:

commit 95e9ac6632cf0515ef511352c6f66485640f8e95
Author: Arild Matsson <[email protected]>
Date:   Tue Jan 30 10:58:56 2024 +0100

    Advanced: never disable "in_order", but show help text, #335

commit 1d73f95d5cc988b511d24e7793ec77f64398437e
Author: Arild Matsson <[email protected]>
Date:   Mon Jan 29 16:51:50 2024 +0100

    Extended: error message if "in order" conflict, #335

commit 04c125cb2c9ae1f6c18f7cc6ac535fd44a3e3882
Author: Arild Matsson <[email protected]>
Date:   Mon Jan 29 15:11:11 2024 +0100

    Disable in_order if query has repetition or boundaries, #335

commit e3aa6687711fbfb4991d89fc97e1b6c905a23bcb
Author: Arild Matsson <[email protected]>
Date:   Mon Jan 29 14:20:54 2024 +0100

    Simple: Update inOrderEnabled more
  • Loading branch information
arildm committed Jan 30, 2024
1 parent 3c3ff72 commit bf5f724
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixed

- Improve UX for "in order" option [#335](https://github.com/spraakbanken/korp-frontend/issues/335)
- Unnecessary scrollbars in the corpus selector info panel [#333](https://github.com/spraakbanken/korp-frontend/issues/333)
- Bug with undefined `arguments`
- On repetition error (all tokens repeat from 0), restore red outline for input
Expand Down
19 changes: 10 additions & 9 deletions app/scripts/components/advanced_search.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,15 @@ export const advancedSearchComponent = {
on-search="$ctrl.onSearch()"
on-search-save="$ctrl.onSearchSave(name)"
></search-submit>
<input id="inOrderChkAdv" type="checkbox" ng-model="$ctrl.inOrder" ng-disabled="!$ctrl.inOrderEnabled" />
<label for="inOrderChkAdv"> {{'in_order_chk' | loc:$root.lang}}</label>
<input id="inOrderChkAdv" type="checkbox" ng-model="$ctrl.inOrder" />
<label for="inOrderChkAdv">
{{'in_order_chk' | loc:$root.lang}}
<i
class="fa fa-info-circle text-gray-400"
uib-tooltip="{{'order_help' | loc:$root.lang}}"
tooltip-placement="right"
></i>
</label>
</div>`,
bindings: {},
controller: [
Expand All @@ -46,8 +53,6 @@ export const advancedSearchComponent = {
const $ctrl = this

$ctrl.inOrder = $location.search().in_order == null
/** Whether the "in order" option is applicable. */
$ctrl.inOrderEnabled = true

if ($location.search().search && $location.search().search.split("|")) {
var [type, ...expr] = $location.search().search.split("|")
Expand All @@ -61,14 +66,10 @@ export const advancedSearchComponent = {
}

$ctrl.onSearch = () => {
// The "in order" option should be available only if >1 token and no wildcards.
const cqpObjs = CQP.parse($ctrl.cqp)
$ctrl.inOrderEnabled = cqpObjs.length > 1 && !CQP.hasWildcards(cqpObjs)

$location.search("search", null)
$location.search("page", null)
$location.search("within", null)
$location.search("in_order", !$ctrl.inOrder && $ctrl.inOrderEnabled ? false : null)
$location.search("in_order", !$ctrl.inOrder ? false : null)
$timeout(() => $location.search("search", `cqp|${$ctrl.cqp}`), 0)
}

Expand Down
40 changes: 32 additions & 8 deletions app/scripts/components/extended/standard_extended.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,22 @@ export const extendedStandardComponent = {
cqp-change="$ctrl.cqpChange(cqp)"
update-repeat-error="$ctrl.updateRepeatError(error)"
></extended-tokens>
<div ng-show="$ctrl.repeatError" style="color: red; margin-bottom: 10px;">
{{'repeat_error' | loc:$root.lang}}
</div>
<div ng-show="$ctrl.orderError" style="color: red; margin-bottom: 10px;">
{{'order_error' | loc:$root.lang}}
</div>
<search-submit
pos="right"
on-search="$ctrl.onSearch()"
on-search-save="$ctrl.onSearchSave(name)"
disabled="$ctrl.repeatError"
disabled="$ctrl.repeatError || $ctrl.orderError"
></search-submit>
<input id="inOrderChkExt" type="checkbox" ng-model="$ctrl.inOrder" ng-disabled="!$ctrl.inOrderEnabled" />
<input id="inOrderChkExt" type="checkbox" ng-model="inOrder" ng-disabled="!$ctrl.inOrderEnabled" />
<label for="inOrderChkExt"> {{'in_order_chk' | loc:$root.lang}}</label>
<span> {{'and' | loc:$root.lang}} </span>
<span>{{'within' | loc:$root.lang}}</span>
Expand All @@ -35,21 +41,23 @@ export const extendedStandardComponent = {
controller: [
"$location",
"$rootScope",
"$scope",
"compareSearches",
"$timeout",
function ($location, $rootScope, compareSearches, $timeout) {
function ($location, $rootScope, $scope, compareSearches, $timeout) {
const ctrl = this

ctrl.lang = $rootScope.lang
ctrl.inOrder = $location.search().in_order == null
$scope.inOrder = $location.search().in_order == null
/** Whether the "in order" option is applicable. */
ctrl.inOrderEnabled = true
ctrl.orderError = false

// TODO this is *too* weird
function triggerSearch() {
$location.search("search", null)
$location.search("page", null)
$location.search("in_order", !ctrl.inOrder && ctrl.inOrderEnabled ? false : null)
$location.search("in_order", !$scope.inOrder && ctrl.inOrderEnabled ? false : null)
$timeout(function () {
$location.search("search", "cqp")
if (!_.keys(settings["default_within"]).includes(ctrl.within)) {
Expand Down Expand Up @@ -87,13 +95,29 @@ export const extendedStandardComponent = {
c.log("Error", e)
}

// The "in order" option should be available only if >1 token and no wildcards.
const cqpObjs = CQP.parse(cqp)
ctrl.inOrderEnabled = cqpObjs.length > 1 && !CQP.hasWildcards(cqpObjs)
ctrl.validateInOrder()

$location.search("cqp", cqp)
}

$scope.$watch("inOrder", () => {
ctrl.validateInOrder()
})

/** Trigger error if the "in order" option is incompatible with the query */
ctrl.validateInOrder = () => {
const cqpObjs = CQP.parse(ctrl.cqp)
if (!CQP.supportsInOrder(cqpObjs)) {
// If query doesn't support free word order, and the "in order" checkbox has been unchecked,
// then disable search, show explanation and let user resolve the conflict
ctrl.orderError = !$scope.inOrder
ctrl.inOrderEnabled = !$scope.inOrder
} else {
ctrl.orderError = false
ctrl.inOrderEnabled = true
}
}

ctrl.cqp = $location.search().cqp

ctrl.repeatError = false
Expand Down
12 changes: 8 additions & 4 deletions app/scripts/components/simple_search.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export const simpleSearchComponent = {

ctrl.inOrder = $location.search().in_order == null
/** Whether the "in order" option is applicable. */
ctrl.inOrderEnabled = true
ctrl.inOrderEnabled = false
ctrl.prefix = $location.search().prefix != null
ctrl.mid_comp = $location.search().mid_comp != null
ctrl.suffix = $location.search().suffix != null
Expand Down Expand Up @@ -233,6 +233,7 @@ export const simpleSearchComponent = {
ctrl.lemgram = search.val
}
$rootScope.simpleCQP = CQP.expandOperators(ctrl.getCQP())
ctrl.updateInOrderEnabled()
ctrl.doSearch()
}
})
Expand All @@ -246,9 +247,12 @@ export const simpleSearchComponent = {
ctrl.currentText = null
}

// The "in order" option should be available only if >1 token.
const cqpObjs = CQP.parse(ctrl.getCQP())
ctrl.inOrderEnabled = cqpObjs.length > 1 && !CQP.hasWildcards(cqpObjs)
ctrl.updateInOrderEnabled()
}

ctrl.updateInOrderEnabled = () => {
const cqpObjs = CQP.parse(ctrl.getCQP() || "[]")
ctrl.inOrderEnabled = CQP.supportsInOrder(cqpObjs)
}

ctrl.doSearch = function () {
Expand Down
19 changes: 11 additions & 8 deletions app/scripts/cqp_parser/cqp.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,15 @@ window.CQP = {
},

/** Check if a query has any wildcards (`[]`) */
hasWildcards(cqpObjs) {
for (const token of cqpObjs) {
// Stringify individual token
const str = CQP.stringify([token])
if (str.indexOf("[]") === 0) return true
}
return false
},
hasWildcard: (cqpObjs) => cqpObjs.some((token) => CQP.stringify([token]).indexOf("[]") === 0),

/** Check if a query has any tokens with repetition */
hasRepetition: (cqpObjs) => cqpObjs.some((token) => token.repeat),

/** Check if a query has any structure boundaries, e.g. sentence start */
hasStruct: (cqpObjs) => cqpObjs.some((token) => token.struct),

/** Determine whether a query will work with the in_order option */
supportsInOrder: (cqpObjs) =>
cqpObjs.length > 1 && !CQP.hasWildcard(cqpObjs) && !CQP.hasRepetition(cqpObjs) && !CQP.hasStruct(cqpObjs),
}
2 changes: 2 additions & 0 deletions app/translations/locale-eng.json
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,8 @@
"add_filter_value": "Add",

"repeat_error": "Error: Not possible to repeat from 0 for all tokens.",
"order_error": "Error: Disabling \"in order\" requires multiple tokens, no repetition, no boundaries and no wildcards.",
"order_help": "Disabling the \"in order\" option is compatible with queries that have multiple tokens, no repetition, no boundaries and no wildcards.",

"choose_value": "Choose a value",
"login_needed_for_corpora": "Login needed for access to",
Expand Down
2 changes: 2 additions & 0 deletions app/translations/locale-swe.json
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,8 @@
"add_filter_value": "Lägg till",

"repeat_error": "Fel: Det går inte att upprepa från 0 på alla token.",
"order_error": "Fel: Det går bara att välja bort \"i följd\" om sökfrågan har flera tokens samt inga repetitioner, gränser eller tomma token.",
"order_help": "Att välja bort \"i följd\" är kompatibelt med sökfrågor som har flera tokens samt inga repetitioner, gränser eller tomma token.",

"choose_value": "Välj ett värde",
"login_needed_for_corpora": "Inloggning behövs för åtkomst till",
Expand Down

0 comments on commit bf5f724

Please sign in to comment.