Pivot
  1. Pivot
  2. PIVOT-744

ListView with SelectMode.Multi does not allow deselection when all elements are selected

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.1
    • Component/s: wtk-terra
    • Labels:
      None

      Description

      If you select all items in a ListView that allows multiple selection, there is not way to deselect using the keyboard. One would expect that releasing shift and moving the arrows would revert to selecting a single element, but this is not the case. The supplied patch allows deselection in the following manner:

      1. If you select all elements with the keyboard "downwards", release shift and then press Keyboard.KeyCode.UP, the "next last" element in the list is selected.
      2. If you select all elements with the keyboard "upwards", release shift and then press Keyboard.KeyCode.DOWN, the "next first" element in the list is selected.

      I tried to take enabled state into account when finding the approproate element to select, so the patch may be more involved than nessecary.

      I tested against ScriptApplication -src=-src=/org/apache/pivot/demos/lists/multi_select.bxml.

      1. pivot744_patch2.patch
        2 kB
        Sandro Martini
      2. TerraListViewSkinDeselect.patch
        3 kB
        Edvin Syse

        Issue Links

          Activity

          Hide
          Edvin Syse added a comment -

          Patch to allow deselection with keyboard when all elements are selected.

          Show
          Edvin Syse added a comment - Patch to allow deselection with keyboard when all elements are selected.
          Hide
          Sandro Martini added a comment - - edited

          It's interesting to see even in other cases if the current behavior has to be fixed (in Tables and Trees) ...

          Show
          Sandro Martini added a comment - - edited It's interesting to see even in other cases if the current behavior has to be fixed (in Tables and Trees) ...
          Hide
          Sandro Martini added a comment -

          patch updated to the current version of the file in trunk

          Show
          Sandro Martini added a comment - patch updated to the current version of the file in trunk
          Hide
          Sandro Martini added a comment -

          excuse me (maybe I've not clear what this patch means) but the desired behavior isn't that if I press <SHIFT> <arrow down> the selection has to be expanded on lower elements (in the list, going bottom) and <SHIFT> <arrow up> has to be expanded upwards ?
          With this patch if I start going down (SHIFT+arrow down) but then I have to deselect the last element I cannot use the opposite (SHIFT+arrow up) because it will expand the selection to the first element over the first selected ... maybe I have not beeen so much clear, but it's simpler to try than to explain.
          At least on Windows should work in this way.

          And in this case why not support even <CTRL> arrow up/down (to add/remove 1 element to the current selection) ?
          Or move this to the 2.1 ... or another 2.0.x ?

          Comments ?

          Show
          Sandro Martini added a comment - excuse me (maybe I've not clear what this patch means) but the desired behavior isn't that if I press <SHIFT> <arrow down> the selection has to be expanded on lower elements (in the list, going bottom) and <SHIFT> <arrow up> has to be expanded upwards ? With this patch if I start going down (SHIFT+arrow down) but then I have to deselect the last element I cannot use the opposite (SHIFT+arrow up) because it will expand the selection to the first element over the first selected ... maybe I have not beeen so much clear, but it's simpler to try than to explain. At least on Windows should work in this way. And in this case why not support even <CTRL> arrow up/down (to add/remove 1 element to the current selection) ? Or move this to the 2.1 ... or another 2.0.x ? Comments ?
          Hide
          Edvin Syse added a comment -

          My patch is NOT changing the behavior you describe, my patch will only change what happens when ALL elements are already selected. I also agree that the current behavior is weird, and not as expected, so a rewrite to the way Windows works for example would probably be wise.

          Show
          Edvin Syse added a comment - My patch is NOT changing the behavior you describe, my patch will only change what happens when ALL elements are already selected. I also agree that the current behavior is weird, and not as expected, so a rewrite to the way Windows works for example would probably be wise.
          Hide
          Greg Brown added a comment -

          Was this patch actually applied? It doesn't look like it - just want to confirm.

          Show
          Greg Brown added a comment - Was this patch actually applied? It doesn't look like it - just want to confirm.
          Hide
          Sandro Martini added a comment -

          No, to avoid side effects (before yesterday's patch on the same classes), but it's here in attach (the newer file).
          If you have time, try to apply and make some test.
          If possible this evening I'll do other tests and commit but for what I've seen all seems to work good with this patch.

          But for changes in behavior (on the current selection, etc via keys) we can see what to do for the 2.1 (should already exist an issue for this).

          Show
          Sandro Martini added a comment - No, to avoid side effects (before yesterday's patch on the same classes), but it's here in attach (the newer file). If you have time, try to apply and make some test. If possible this evening I'll do other tests and commit but for what I've seen all seems to work good with this patch. But for changes in behavior (on the current selection, etc via keys) we can see what to do for the 2.1 (should already exist an issue for this).
          Hide
          Greg Brown added a comment -

          OK. If you do apply this patch and it works correctly, you should be sure to make similar changes to TableView and TreeView.

          Show
          Greg Brown added a comment - OK. If you do apply this patch and it works correctly, you should be sure to make similar changes to TableView and TreeView.
          Hide
          Sandro Martini added a comment -

          Ok, today I'll make some other tests and today/in a few days I'll commit changes in ListView class and only after in related classes (I wasn't sure before this discussion that the current behavior was right).

          Show
          Sandro Martini added a comment - Ok, today I'll make some other tests and today/in a few days I'll commit changes in ListView class and only after in related classes (I wasn't sure before this discussion that the current behavior was right).
          Hide
          Sandro Martini added a comment -

          Patch applied on TerraListViewSkin and tested against tests/ ... / multiple_selection_list.bxml .
          Now I have to reproduce the same behavior even on Tables and Trees before closing this issue.

          In the meantime, many thanks to Edvin for the help .

          Show
          Sandro Martini added a comment - Patch applied on TerraListViewSkin and tested against tests/ ... / multiple_selection_list.bxml . Now I have to reproduce the same behavior even on Tables and Trees before closing this issue. In the meantime, many thanks to Edvin for the help .
          Hide
          Sandro Martini added a comment -

          Resolved on ListView, and moved in the other ticked for TableView and TreeView

          Show
          Sandro Martini added a comment - Resolved on ListView, and moved in the other ticked for TableView and TreeView

            People

            • Assignee:
              Sandro Martini
              Reporter:
              Edvin Syse
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 0.25h
                0.25h
                Remaining:
                Remaining Estimate - 0.25h
                0.25h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development