Code review comment for lp:~matthew-constable/tiqit/tiqit

Revision history for this message
Matthew Hall (matt.hall) wrote :

This is a good change, but unfortunately there's currently inconsistent behaviour displayed when search results are grouped by one of the fields.

* When a column is added using the buttons above any group, it is added to all groups and the URL updated. I think this is fine.

* When a column is removed using the buttons above any group but the first, it is removed from only that group and the URL isn't updated. The existing behaviour of the group isn't ideal, but at least your new URL handling is being consistent with it.

* When a column is remove using the buttons above the first group, it is removed from only that group and the URL *is* updated. Again, the existing grouping behavior arguably isn't ideal, but now there's also an inconsistency between the union of all columns displayed on the page and the columns listed in the URL.

We either need to avoid updating the URL when removing a column from the first group, or change the grouping behavior so that removing a column always affects all groups.

review: Needs Fixing

« Back to merge proposal