Pivot
  1. Pivot
  2. PIVOT-694

Improvement in ListButton clear() method

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.1
    • Component/s: wtk
    • Labels:
      None
    • Environment:
      Pivot 2.0
      Win 7

      Description

      I'm having problems with ListButton.
      When I try to clean a ListButton the last text selected before remains selected, even with no item in the ListData of the ListButton.
      I tried listButton.clear() and listButton.getListData().clear(), but with none of this I can scape the problem.
      I debugged the application, and the listData of the ListButton is clear after getListData().clear(), but the last selected text remains showing.
      See attached image.

      Perhaps listButton.setButtonData(null) should be performed in ListButton#clear().

      Cheers

      1. listButton problem.png
        67 kB
        Luiz Gustavo Stábile de Souza

        Issue Links

          Activity

          Hide
          Luiz Gustavo Stábile de Souza added a comment -

          This image shows the state of the ListButton before and after the listData#getListData().clear() method call.

          Show
          Luiz Gustavo Stábile de Souza added a comment - This image shows the state of the ListButton before and after the listData#getListData().clear() method call.
          Hide
          Luiz Gustavo Stábile de Souza added a comment -

          Additionally, listButton#clear() seems not to clear the listData. Isn't that one of the objectives of calling listButton#clear(), or only listButton#getListData().clear() should do that?

          Show
          Luiz Gustavo Stábile de Souza added a comment - Additionally, listButton#clear() seems not to clear the listData. Isn't that one of the objectives of calling listButton#clear(), or only listButton#getListData().clear() should do that?
          Hide
          Sandro Martini added a comment -

          Hi, in my Test application (Pivot694, the latest version has to be committed), with the following patch (reported here) seem to solve the problem.

          Someone can confirm to me that this new behavior could be right (if needed I could commit it as soon as possible) ?
          Telol me something, thank you very much.

          Note: the latest verion of Pivot694 has to be committed because I'm using it to verify what the clear() method on mahy components do, to have more consistency between components.

          Bye,
          Sandro

              1. Eclipse Workspace Patch 1.0
                #P wtk
                Index: src/org/apache/pivot/wtk/ListButton.java
                ===================================================================
              • src/org/apache/pivot/wtk/ListButton.java (revision 1142425)
                +++ src/org/apache/pivot/wtk/ListButton.java (working copy)
                @@ -772,6 +772,9 @@
                if (selectedItemKey != null) { setSelectedItem(null); }

                +
                + if (selectedIndex > -1)
                + setSelectedIndex(-1);
                }

          /**

          Show
          Sandro Martini added a comment - Hi, in my Test application (Pivot694, the latest version has to be committed), with the following patch (reported here) seem to solve the problem. Someone can confirm to me that this new behavior could be right (if needed I could commit it as soon as possible) ? Telol me something, thank you very much. Note: the latest verion of Pivot694 has to be committed because I'm using it to verify what the clear() method on mahy components do, to have more consistency between components. Bye, Sandro Eclipse Workspace Patch 1.0 #P wtk Index: src/org/apache/pivot/wtk/ListButton.java =================================================================== src/org/apache/pivot/wtk/ListButton.java (revision 1142425) +++ src/org/apache/pivot/wtk/ListButton.java (working copy) @@ -772,6 +772,9 @@ if (selectedItemKey != null) { setSelectedItem(null); } + + if (selectedIndex > -1) + setSelectedIndex(-1); } /**
          Hide
          Greg Brown added a comment -

          The issue is that the button data is not being cleared in response to a list data change. The attached patch does not resolve that issue.

          Show
          Greg Brown added a comment - The issue is that the button data is not being cleared in response to a list data change. The attached patch does not resolve that issue.
          Hide
          Sandro Martini added a comment -

          Hi Greg, thanks for the help ... you have reason, I was more focused only on a "clearSelection" but probably for this there is already the setSelectedIndex(-1) method in many components ... but I'm guessing if this could be useful in ButtonGroup/RadioButtonGroup for example but this is another story.
          Maybe in my new Pivot694, add button for "Clear Selection" in all components, and another for "Clear Data", to test both behaviors.

          I'll try to reproduce the condition where clear means delete all data inside the element, and see what happens.
          Then in other components, to see what happens.

          Bye

          Show
          Sandro Martini added a comment - Hi Greg, thanks for the help ... you have reason, I was more focused only on a "clearSelection" but probably for this there is already the setSelectedIndex(-1) method in many components ... but I'm guessing if this could be useful in ButtonGroup/RadioButtonGroup for example but this is another story. Maybe in my new Pivot694, add button for "Clear Selection" in all components, and another for "Clear Data", to test both behaviors. I'll try to reproduce the condition where clear means delete all data inside the element, and see what happens. Then in other components, to see what happens. Bye
          Hide
          Greg Brown added a comment -

          I just checked in what should be a fix for this. Please re-open if the problem persists.

          Show
          Greg Brown added a comment - I just checked in what should be a fix for this. Please re-open if the problem persists.
          Hide
          Sandro Martini added a comment -

          Hi Greg, thank you very much for the fix, I'll try during next days ... and I'll try even on other components to verify the same behavior.

          On the "clearSelection" on ButtonGroup/RadioButtonGroup what do you think ?
          I could put this in a dedicated ticket (maybe for the 2.1) if you think could be useful to have.

          Bye

          Show
          Sandro Martini added a comment - Hi Greg, thank you very much for the fix, I'll try during next days ... and I'll try even on other components to verify the same behavior. On the "clearSelection" on ButtonGroup/RadioButtonGroup what do you think ? I could put this in a dedicated ticket (maybe for the 2.1) if you think could be useful to have. Bye
          Hide
          Greg Brown added a comment -

          "I'll try even on other components to verify the same behavior."

          This fix only applies to ListButton.

          Show
          Greg Brown added a comment - "I'll try even on other components to verify the same behavior." This fix only applies to ListButton.
          Hide
          Sandro Martini added a comment -

          >This fix only applies to ListButton.
          I understand this, I'm thinking to make similar tests on other components that contains some data and could be selected, and see after something has been selected, if clear reset even the selection ... for example in ListView/TableView/TreeView/etc.

          >On the "clearSelection" on ButtonGroup/RadioButtonGroup what do you think ?
          For example in for example ListView/TableView/TreeView it is present, so why not even there ...
          >I could put this in a dedicated ticket (maybe for the 2.1) if you think could be useful to have.
          Sorry, but you didn't give me a comment on this ... what do you think ?

          Thanks again.

          Show
          Sandro Martini added a comment - >This fix only applies to ListButton. I understand this, I'm thinking to make similar tests on other components that contains some data and could be selected, and see after something has been selected, if clear reset even the selection ... for example in ListView/TableView/TreeView/etc. >On the "clearSelection" on ButtonGroup/RadioButtonGroup what do you think ? For example in for example ListView/TableView/TreeView it is present, so why not even there ... >I could put this in a dedicated ticket (maybe for the 2.1) if you think could be useful to have. Sorry, but you didn't give me a comment on this ... what do you think ? Thanks again.
          Hide
          Sandro Martini added a comment -

          Hi Greg,
          after committing my (updated, and future-proof) Pivot-694 Test Application (and related bxml), I think the issue is still open, or I have to me missed something.

          The change you made in rev r1151475 in ListButtonSkin.listDataChanged() is called only at startup.

          Can you try my test app (the Clear data button) ?
          Tell me, and thank you in the meantime.

          If others have comments, post here.

          Bye

          Show
          Sandro Martini added a comment - Hi Greg, after committing my (updated, and future-proof) Pivot-694 Test Application (and related bxml), I think the issue is still open, or I have to me missed something. The change you made in rev r1151475 in ListButtonSkin.listDataChanged() is called only at startup. Can you try my test app (the Clear data button) ? Tell me, and thank you in the meantime. If others have comments, post here. Bye
          Hide
          Greg Brown added a comment -

          It should be called any time the list data changes. It won't be triggered by a clear() unless you have mapped a key to the list data.

          Show
          Greg Brown added a comment - It should be called any time the list data changes. It won't be triggered by a clear() unless you have mapped a key to the list data.
          Hide
          Sandro Martini added a comment - - edited

          Greg, your patch has fixed the original problem of this ticket.
          But if you try my (just updated Pivot694.java, and related bxml file), you can see some other issues, look at some TODO inside the source, we can speak on them tomorrow (probably there is something wrong in my test, but it's better to be sure). Now I try the clearoperation as you suggested on many Pivot Components there.

          Note that one time this issue will be resolved probably makes sense to move Pivoy694 (java and bxml) one level upper on issues (as package) and rename in something like ClearTest, could be useful, at least as a sample.

          Bye

          Show
          Sandro Martini added a comment - - edited Greg, your patch has fixed the original problem of this ticket. But if you try my (just updated Pivot694.java, and related bxml file), you can see some other issues, look at some TODO inside the source, we can speak on them tomorrow (probably there is something wrong in my test, but it's better to be sure). Now I try the clearoperation as you suggested on many Pivot Components there. Note that one time this issue will be resolved probably makes sense to move Pivoy694 (java and bxml) one level upper on issues (as package) and rename in something like ClearTest, could be useful, at least as a sample. Bye
          Hide
          Sandro Martini added a comment -

          Greg (and others), I try to write here a short summary of things to make this issue resolved, at least for ListButton (currently the same issue happens even in other components, and this is the reason because I haven't resolved this issue now):

          to have an event after a clear on a component like ListButton, you have to set (for example in bxml) the attribute listDataKey (other components have similar names) a value for it. Then (when data has to be resetted in the component) call its clear() method, BUT after call even its repaint() method ... is this right ?

          Hope to be clear.

          Note: I haven't found a sample for this in all Pivot, so could be useful to move related test sources to something like ClearTest (under tests subproject one time this issue will be completely resolved).
          And for 2.1 extend this for clearSelection etc ... but this will be handled in another ticket.

          Comments ?

          Show
          Sandro Martini added a comment - Greg (and others), I try to write here a short summary of things to make this issue resolved, at least for ListButton (currently the same issue happens even in other components, and this is the reason because I haven't resolved this issue now): to have an event after a clear on a component like ListButton, you have to set (for example in bxml) the attribute listDataKey (other components have similar names) a value for it. Then (when data has to be resetted in the component) call its clear() method, BUT after call even its repaint() method ... is this right ? Hope to be clear. Note: I haven't found a sample for this in all Pivot, so could be useful to move related test sources to something like ClearTest (under tests subproject one time this issue will be completely resolved). And for 2.1 extend this for clearSelection etc ... but this will be handled in another ticket. Comments ?
          Hide
          Greg Brown added a comment -

          This issue has nothing to do with repaint() - it only applies to setListData() and clear().

          Show
          Greg Brown added a comment - This issue has nothing to do with repaint() - it only applies to setListData() and clear().
          Hide
          Sandro Martini added a comment -

          >This issue has nothing to do with repaint() - it only applies to setListData() and clear().
          Probably I have not been so clear ... after your fix and my last commits, if you run Pivot964.java you can see that many components in the test application have two versions: one with listDataKey/xxxdataKey set and another not, in the same row.
          Clicking the "Clear Data" button you can see that all components with the xxxDataKey set should have been emptied, but:

          • without force a repaint() in (test application) code this is not achieved, so the question to you:is it right to force a repaint in my code there ? For what I've been looking in other Pivot sources seems yes ...
          • even with forcing a repaint, in some components I don't see the "empty" content in them, and this is the remaining part to fix for this ticket. Maybe this is a little "corner case", but I'd prefer to finish fixing it for the 2.0.1, do you agree ?

          In my previous comment here I was trying to write a mini-how-to on how to make this use case work in user applications, could be useful to someone until I'll move Pivot694* test files in a more useful names, all here.

          Bye

          Show
          Sandro Martini added a comment - >This issue has nothing to do with repaint() - it only applies to setListData() and clear(). Probably I have not been so clear ... after your fix and my last commits, if you run Pivot964.java you can see that many components in the test application have two versions: one with listDataKey/xxxdataKey set and another not, in the same row. Clicking the "Clear Data" button you can see that all components with the xxxDataKey set should have been emptied, but: without force a repaint() in (test application) code this is not achieved, so the question to you:is it right to force a repaint in my code there ? For what I've been looking in other Pivot sources seems yes ... even with forcing a repaint, in some components I don't see the "empty" content in them, and this is the remaining part to fix for this ticket. Maybe this is a little "corner case", but I'd prefer to finish fixing it for the 2.0.1, do you agree ? In my previous comment here I was trying to write a mini-how-to on how to make this use case work in user applications, could be useful to someone until I'll move Pivot694* test files in a more useful names, all here. Bye
          Hide
          Sandro Martini added a comment -

          Resolved on ListButtons, and moved in another ticket the part related to other components.

          Show
          Sandro Martini added a comment - Resolved on ListButtons, and moved in another ticket the part related to other components.

            People

            • Assignee:
              Greg Brown
              Reporter:
              Luiz Gustavo Stábile de Souza
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development