ListGrid 'Clear Filter' does not clear criteria on hidden column

[cyuen]

The 'Clear Filter' menu item from the FilterEditor of the ListGrid does not clear the filter if a column is hidden.

Problem can be reproduced on the SmartClient Adaptive Filter example on http://smartclient.com/#adaptiveFilterFS using SmartClient Version: v9.1p_2014-07-13/Pro Deployment (built 2014-07-13) on Firefox 31.0 and Chrome 36.0

Steps to reproduce:

1) Open the Adaptive Filter example on http://smartclient.com/#adaptiveFilterFS

2) Enter a value (e.g. '45300') in the SKU filter editor and press Enter
=> The list is filtered according to the filter

3) Right click on the header and select Column menu item from the popup menu to hide the SKU column without clearing the filter
=> The SKU column is hidden

4) Right click on the header and select 'Clear Filter' menu item to clear the menu
ERROR> Nothing happens, the filtered results is still displayed. Expecting the filter to be cleared and displays the full list

5) Right click on the header and select Column menu item from the popup menu to show the SKU column again
ERROR> The filter value entered in step 2) is still in the filter editor. Expecting all criterion on filter editor to be empty

It seems that when 'Clear Filter' is invoked, ListGrid.filterEditor._editRowForm.values are not cleared when the column is hidden (not in _editRowForm.items anymore) and these values are then being overlay with criteria from the visible items to form the search criteria when ListGrid.getFilterEditorCriteria() is called.

----------------
SmartClient Version: v9.1p_2014-07-13/Pro Deployment (built 2014-07-13)
Windows 7 Firefox 31.0 and Chrome 36.0.1985.125 m

[Isomorphic]

Thanks for the report. This issue is under investigation. We'll follow up when we have more information

Regards
Isomorphic Software

[cyuen]

Thanks for the response. Would you be able to let us know when a fix will be available?

This is new behavior after upgrading to SmartClient 9.1 and it is causing issues in our application. It was working properly when using SmartClient 8.3.

The SmartClient 8.3 Adaptive Filter example on http://www.smartclient.com/docs/8.3/...aptiveFilterFS also has no issue.

'Clear Filter' clears the filter editor on the hidden columns as well (e.g. SKU column in the above example).

[Isomorphic]

We've made a change to address this issue in both 9.1 and 10.0
Please try the next nightly build (dated Aug 7 or above)

Regards
Isomorphic Software

[cyuen]

Thanks Isomorphic. The new build resolves this issue.

[cyuen]

Not working with AdvancedCriteria

This is found using SmartClient Version: v9.1p_2014-08-08/Pro Deployment (built 2014-08-08) on Firefox 31.0 and Chrome 36.0

The 'Clear Filter' fix provided in build Aug 7 or above only addresses the issue when the criteria is null or is a simple criteria. However, it does not work when the criteria is an AdvancedCriteria. This happens when ListGrid.initialCritieria is set and it is then combined with the criteria from the filterEditor to form an AdvancedCriteria.

It seems like in isc.DynamicForm.setValuesAsCriteria(), it is clearing the criteria for visible items only for advanced criteria in the else clause.

----------------
SmartClient Version: v9.1p_2014-08-08/Pro Deployment (built 2014-08-08)
Windows 7 Firefox 31.0 and Chrome 36.0.1985.125 m

[Isomorphic]

This is starting to be a bit strange from a UI perspective.

The user cannot see the initialCriteria in the FilterEditor, and most UIs use initialCriteria or other non-editable criteria for things that should never be user-cleared.

For example, you may have a grid showing details of a particular account, including a searchable grid containing orders for a particular account. The account ID is implicit criteria in the grid and should not be cleared by "Clear Filter".

So the default behavior here is definitely right. If you can explain how you are ending up in a situation where it makes sense for the user to clear criteria he can't edit, we can suggest an approach.

[cyuen]

Yes I agree the initialCriteria should never be user-cleared.

For example, assuming we are using the same SmartClient Adaptive Filter example on http://smartclient.com/#adaptiveFilterFS and the ListGrid is showing a particular account with an advanced initial criteria (e.g. account# == 001 && dept# == 003).

And from this list if we perform:

1) Enter a value (e.g. '45300') in the SKU filter editor in the SKU column and press Enter
=> The list is filtered according to the criteria from filter editor and initial criteria (i.e. account# 001 && dept# 003 && SKU contains 45300)

2) Right click on the header and select Column menu item from the popup menu to hide the SKU column without clearing the filter
=> The SKU column is hidden

3) Right click on the header and select 'Clear Filter' menu item to clear the menu
ERROR> Nothing happens, the filtered results is still displayed. Expecting the filter to be cleared and displays the full list.

The criteria for account# and dept# (initial criteria) should never be cleared but the criteria for SKU (from hidden column) should be cleared.

Debugging the SmartClient source code, it seems like in isc.DynamicForm.setValuesAsCriteria(), it is clearing the criteria only for visible items for advanced criteria in the else clause; whereas it is dropping the extra criteria for hidden column/items as well in the simple criteria case (if clause) as in isc.DynamicForm._saveValuesAsCriteria()

[Isomorphic]

The behavior your describing is exactly what we have recently changed. It seems like this should be working correctly (clearing the value from the hidden field). Can you verify that you're running with the latest nightly build (perhaps post the result of evaluating "isc.version" in the developer console to be sure)

Thanks

[cyuen]

I am currently running SmartClient Version: v9.1p_2014-08-08/Pro Deployment (built 2014-08-08)

> isc.version
"v9.1p_2014-08-08/Pro Deployment"
>

The clearing works if there is no initial criteria or the initial criteria is simple Criterion.

If the initial criteria is AdvancedCriteria, it does not work because it goes thru a different code path in isc.DynamicForm.setValuesAsCriteria() which is using getItems() (visible items) to check against the criteria only.

setValuesAsCriteria : function (criteria, advanced, dropExtraCriteria, dropCriteriaFields) {

    if (!advanced && !isc.DataSource.isAdvancedCriteria(criteria)) {
        // In this case the criteria passed in is a simple values object of fieldName-> value
        // mappings.
        // We could just do 'setValues(criteria)' and it would work in most cases, however we
        // support having items work with simple criteria but use a different criteria field
        // (EG ComboBoxItem with display field set and addUnknownValues:true).
        // Therefore we want to actually go through all our items and allow them to grab specific
        // criteria they're interested in.

        this._saveValuesAsCriteria(criteria, dropExtraCriteria, dropCriteriaFields);
        var items = this.items || [];
        var itemsToClear = [];


        if (dropExtraCriteria && (!dropCriteriaFields || dropCriteriaFields.length == 0)) {
            delete this._extraAdvancedCriteria;
        } else if (this._extraAdvancedCriteria) {
            if (this._parseExtraCriteria) {
                // RecordEditor uses this - remove any entries in the stored
                // _extraAdvancedCriteria that do not appear in the new criteria
                this.removeExtraAdvancedCriteria(criteria);
                var eAC = this._extraAdvancedCriteria;

                if (dropExtraCriteria && dropCriteriaFields) {
                    this.removeExtraAdvancedCriteriaFields(dropCriteriaFields);
                }

                if (!eAC || !eAC.criteria || eAC.criteria.length == 0) eAC = null;
            } else {
                // normal forms just clear out any stored extra criteria
                delete this._extraAdvancedCriteria;
            }
        }

        for (var i = 0; i < items.length; i++) {
            var item = items[i],
                itemName = item.getFieldName(),
                itemModified = false;
            if (isc.propertyDefined(criteria, itemName) && item.canEditSimpleCriterion(itemName)) {
                item.setSimpleCriterion(criteria[itemName], itemName);
                itemModified = true;
            } else {
                for (var fieldName in criteria) {
                    if (fieldName != itemName && item.canEditSimpleCriterion(fieldName)) {
                        item.setSimpleCriterion(criteria[fieldName], fieldName);
                        itemModified = true;
                        break;
                    }
                }
            }
            if (!itemModified) {
                itemsToClear.add(item);
            }
        }

        // Explicitly empty any items we didn't touch
        for (var i = 0; i < itemsToClear.length; i++) {
            if (!itemsToClear[i].shouldSaveValue) continue;
            itemsToClear[i].clearValue();
        }

        this.rememberValues();
    } else {

        // Wipe out any existing "values" object.
        // We'll update the values for each item that can edit subcriterion of the criteria
        // passed in below, which will also store their simple value in the values object,
        // but this ensures we don't hang onto values for stale keys.

        var oldValues = this.values;
        this._saveValues({});

        // copy the crit object - we don't want to directly manipulate it and confuse other
        // code
        criteria = isc.clone(criteria);

        var topOperator = criteria.operator;
        if (topOperator != this.operator) {
            // this doesn't necessarily indicate an error but it might be unexpected.
            // Log a warning and wrap in a top level AC object.
            this.logInfo("Dynamic Form editing advanced criteria object:" +
                isc.Comm.serialize(criteria) + ". Form level operator specified as '" +
                this.operator + "' - Criteria returned from this form will be nested in an outer " +
                this.operator + " clause.", "AdvancedCriteria");

            criteria._constructor = null;
            criteria = {
                _constructor:"AdvancedCriteria",
                operator:this.operator,
                criteria:[criteria]
            }
        }

        // We have to determine which items will edit which of the criteria.
        // For each inner criterion - see if we have an item that can edit it. If so,
        // clear it off the stored "extra criteria" and apply it directly to the item for
        // editing. getValuesAsCriteria() will reconstitute it when it runs!
        // Note: Some items have the ability to edit composite ("and" / "or") criteria - for
        // example if editing expressions a user can enter ">1 and <2".
        // This means we can't assume a 1:1 mapping between top level criterion objects and
        // items - we may have to combine multiple top level criteria acting on a particular field
        // into a single composite criterion and apply this to an item.
        // getValuesAsCriteria() simplifies criteria down so we don't need to worry about introducing
        // extra levels of nesting - the returned criteria will be logically equivalent and as
        // simple as possible.

        var items = this.getItems(),
            innerCriteria = criteria.criteria,
            assigned = {},
            itemsToClear = {};

        for (var i = 0; i < items.length; i ++) {
            itemsToClear[items[i].getID()] = true;
        }

        for (var i = 0; i < innerCriteria.length; i++) {

            for (var ii = 0; ii < items.length; ii++) {
                if (!items[ii].shouldSaveValue) {
                    itemsToClear[items[ii].getID()] = false;
                    continue;
                }
                var item = items[ii];

                if (this.shouldApplyCriterionToItem(items[ii], innerCriteria[i])) {
//                      this.logWarn("applying advanced criterion:" + isc.Comm.serialize(innerCriteria[i]) +
//                          "to item:" + items[ii]);
                    var itemID = items[ii].getID();
                    if (assigned[itemID] == null) {
                        assigned[itemID] = innerCriteria[i];
                        itemsToClear[itemID] = false;
                    } else {
                        // Do not try to combine criteria for items that express canEditOpaqueValues
                        if (!items[ii].canEditOpaqueValues) {
                            var existingCriteria = assigned[itemID];
                            var compositeCriterion = isc.DataSource.combineCriteria(
                                existingCriteria, innerCriteria[i],

                                this.operator, null, true);


                            if (!item.canEditCriterion(compositeCriterion)) {
                                this.logInfo("setValuesAsCriteria(): criteria include:" +
                                    this.echoFull(existingCriteria) + " and " +
                                    this.echoFull(innerCriteria[i]) + ". Both of these " +
                                    "could be applied to item:" + item +
                                    ". However, the item is unable to edit a composite criterion " +
                                    "resulting from combining these criteria. Therefore " +
                                    this.echoFull(innerCriteria[i]) + " will not be applied to this item",
                                    "AdvancedCriteria");

                                // Don't clear the inner criteria - we'll see if another item can
                                // edit it, otherwise we'll leave it around as "extraAdvancedCriteria"
                                continue;

                            } else {
                                this.logDebug("setValuesAsCriteria(): Combined multiple criteria into " +
                                    "composite criterion:" +
                                    this.echoFull(compositeCriterion) + " and assigned to item:" + item,
                                    "AdvancedCriteria");
                                assigned[itemID] = compositeCriterion;
                                itemsToClear[itemID] = false;
                            }
                        } else {
                            // Leave it around as "extraAdvancedCriteria"
                            continue;
                        }
                    }
                    innerCriteria[i] = null;
                    // no need to go through the rest of the items for this criterion...
                    break;
//                 } else {
//                     this.logWarn("Not applying adv criterion:"
//                      + isc.Comm.serialize(innerCriteria[i]) + " to item:" + items[ii]);
                }


            }
        }
        innerCriteria.removeEmpty();

        // actually call 'setCriterion' to apply the criteria to the items
        for (var itemID in assigned) {
            var item = window[itemID];
            var value = assigned[itemID];
            if (item.canEditOpaqueValues && value) {
                isc.Canvas._saveFieldValue(null, item, value.value, oldValues, this, true, "criteria");
                value.value = isc.Canvas._getFieldValue(null, item, oldValues, this, true, "edit");
            }
            item.setCriterion(value);
        }

        // Clear any editable fields that aren't editing anything specific in the criterion.
        for (var itemID in itemsToClear) {
            if (!itemsToClear[itemID]) continue;
            var item = window[itemID];
            item.clearValue();
        }
        // store the fields we're not directly editing -- these will be recombined with
        // live values as part of getValuesAsCriteria();
        this._extraAdvancedCriteria = criteria;
    }
},

[Isomorphic]

A change was made since the 8/8 build you're using to address this. We just ran a sanity check and this is working as expected for us.
Please get the latest nightly and let us know if this persists.

Thanks
Isomorphic Software