Pivot
  1. Pivot
  2. PIVOT-765

Reintroduce WindowStateListener#previewWindowOpen

    Details

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

      Description

      To support dynamic creation of the Menu for a MenuButton, the previewWindowOpen could be reintroduced so that one can populate the Menu before the popup window is opened. Today, one can only populate the menu in WindowStateListener#windowOpened, ie. after the popup window is displayed. This is problematic because correct positioning and sizing cannot be determined before the menu is populated. There are probably other use cases as well. This one was discussed here:

      http://apache-pivot-users.399431.n3.nabble.com/How-to-create-menu-for-MenuButton-dynamically-td3103392.html

      Code example:

      menuButton.getListPopup().getWindowStateListeners().add(new WindowStateListener.Adapter() {
      public Vote previewWindowOpen(Window window)

      { // Populate menuButton's menu return Vote.APPROVE; }

      }

      I will attach a patch that adds said functionality.

        Activity

        Hide
        Greg Brown added a comment - - edited

        You may also want to reintroduce the opening flag. See the 1.5 version of Window for details.

        Show
        Greg Brown added a comment - - edited You may also want to reintroduce the opening flag. See the 1.5 version of Window for details.
        Hide
        Edvin Syse added a comment -

        New patch that also reintroduces the opening flag.

        Show
        Edvin Syse added a comment - New patch that also reintroduces the opening flag.
        Hide
        Sandro Martini added a comment -

        Hi Edvin,
        have you got commit right now ?

        Have you some time to write a simple test application, so is will be easier to test the correct behavior of this new feature ?
        Many thanks ...

        Bye

        Show
        Sandro Martini added a comment - Hi Edvin, have you got commit right now ? Have you some time to write a simple test application, so is will be easier to test the correct behavior of this new feature ? Many thanks ... Bye
        Hide
        Edvin Syse added a comment -

        Waiting for the commit rights, but I absolutely have time to write the test. Can you show me a similar test app so I can see how you guys would write it, or give me some pointers?

        Show
        Edvin Syse added a comment - Waiting for the commit rights, but I absolutely have time to write the test. Can you show me a similar test app so I can see how you guys would write it, or give me some pointers?
        Hide
        Sandro Martini added a comment -

        Good ... on similar test (minimal) applications, you can look at sources under tests\src\org\apache\pivot\tests\issues ,
        or anything other under tests, as you prefer.
        Only one thing, because with applications it's not so simple to see if the behavior is as desired (after months that has written, maybe from another developer) ... I can suggest to put some info on the right behavior as comment in sources, or better (in my opinion) as System.out.println() so any user can read and verify , but it's only a suggestion.

        Bye

        Show
        Sandro Martini added a comment - Good ... on similar test (minimal) applications, you can look at sources under tests\src\org\apache\pivot\tests\issues , or anything other under tests, as you prefer. Only one thing, because with applications it's not so simple to see if the behavior is as desired (after months that has written, maybe from another developer) ... I can suggest to put some info on the right behavior as comment in sources, or better (in my opinion) as System.out.println() so any user can read and verify , but it's only a suggestion. Bye
        Hide
        Edvin Syse added a comment -

        This test will show an Alert when you click the MenuButton unless previewWindowOpened is called first. If you apply this test-patch before the actual previewWindowOpen patch you should see the Alert. After applying the actual patch, you should see a dynamically created menu item. Is this satisfactory?

        Show
        Edvin Syse added a comment - This test will show an Alert when you click the MenuButton unless previewWindowOpened is called first. If you apply this test-patch before the actual previewWindowOpen patch you should see the Alert. After applying the actual patch, you should see a dynamically created menu item. Is this satisfactory?
        Hide
        Sandro Martini added a comment -

        This requires more tests, so move to 2.0.2

        Show
        Sandro Martini added a comment - This requires more tests, so move to 2.0.2
        Hide
        Edvin Syse added a comment -

        Can you hint on what else should be tested? I'd be happy to write more tests.

        Show
        Edvin Syse added a comment - Can you hint on what else should be tested? I'd be happy to write more tests.
        Hide
        Sandro Martini added a comment -

        Hi Edvin,
        to be sure that this doesn't cause problems to existing behavior, can you commit a minimal test application (maybe under tests, with others) ?
        Thank you.

        Bye

        Show
        Sandro Martini added a comment - Hi Edvin, to be sure that this doesn't cause problems to existing behavior, can you commit a minimal test application (maybe under tests, with others) ? Thank you. Bye
        Hide
        Sandro Martini added a comment -

        Reassigned, so could be another thing fixed/improved for 2.0.1

        Show
        Sandro Martini added a comment - Reassigned, so could be another thing fixed/improved for 2.0.1
        Hide
        Sandro Martini added a comment -

        Hi Edvin,
        thanks for your patience ...

        I verified with Roger and this fix seems to not give problems to other applications ... so go with the commit all files for 2.0.1 .
        Or tell me if you haven't time to do it soon ...

        Bye

        Show
        Sandro Martini added a comment - Hi Edvin, thanks for your patience ... I verified with Roger and this fix seems to not give problems to other applications ... so go with the commit all files for 2.0.1 . Or tell me if you haven't time to do it soon ... Bye
        Hide
        Edvin Syse added a comment -

        Just committed with a small test application.

        Show
        Edvin Syse added a comment - Just committed with a small test application.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development