ICEfaces
  1. ICEfaces
  2. ICE-5520

PanelPopup not auto-centered when using positionOnLoadOnly="true"

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.2
    • Fix Version/s: 1.8.2-EE-GA_P01, 1.8.3
    • Component/s: ICE-Components
    • Labels:
      None
    • Environment:
      All

      Description

      When using draggable="true", autoCentre="true" and positionOnLoadOnly="true", the popup is not centered.

        Activity

        Hide
        yip.ng added a comment -

        See ICE-5496 for additional comments.

        Show
        yip.ng added a comment - See ICE-5496 for additional comments.
        Hide
        yip.ng added a comment -

        Some code in PanelPopupRenderer was replicated from GroupRenderer. When changes to GroupRenderer were made for ICE-4695, they were not made correspondingly in PanelPopupRenderer, causing some bugs.

        Show
        yip.ng added a comment - Some code in PanelPopupRenderer was replicated from GroupRenderer. When changes to GroupRenderer were made for ICE-4695 , they were not made correspondingly in PanelPopupRenderer, causing some bugs.
        Hide
        yip.ng added a comment -

        Also some bug in auto centering when dialog is modal.

        Show
        yip.ng added a comment - Also some bug in auto centering when dialog is modal.
        Hide
        yip.ng added a comment -

        Fixed. See video of testing at http://screencast.com/t/M2EzMWJiMGQt.

        Tested with markup as shown in the forum:

        <ice:panelPopup id="modalPnlPop"
        draggable="true" modal="true"
        positionOnLoadOnly="true"
        visible="#

        {popup.modalRendered}

        "
        autoCentre="true"
        styleClass="corePopup">

        Show
        yip.ng added a comment - Fixed. See video of testing at http://screencast.com/t/M2EzMWJiMGQt . Tested with markup as shown in the forum: <ice:panelPopup id="modalPnlPop" draggable="true" modal="true" positionOnLoadOnly="true" visible="# {popup.modalRendered} " autoCentre="true" styleClass="corePopup">
        Hide
        yip.ng added a comment -

        Revision: 21052


        Modified : /icefaces/trunk/icefaces/bridge/lib/extras/style.js
        Modified : /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/ext/renderkit/GroupRenderer.java
        Modified : /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/panelpopup/PanelPopupRenderer.java

        Revision: 21053


        Modified : /icefaces-ee/branches/icefaces-ee-1.8.2_p01/icefaces/bridge/lib/extras/style.js
        Modified : /icefaces-ee/branches/icefaces-ee-1.8.2_p01/icefaces/component/src/com/icesoft/faces/component/ext/renderkit/GroupRenderer.java
        Modified : /icefaces-ee/branches/icefaces-ee-1.8.2_p01/icefaces/component/src/com/icesoft/faces/component/panelpopup/PanelPopupRenderer.java

        Show
        yip.ng added a comment - Revision: 21052 Modified : /icefaces/trunk/icefaces/bridge/lib/extras/style.js Modified : /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/ext/renderkit/GroupRenderer.java Modified : /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/panelpopup/PanelPopupRenderer.java Revision: 21053 Modified : /icefaces-ee/branches/icefaces-ee-1.8.2_p01/icefaces/bridge/lib/extras/style.js Modified : /icefaces-ee/branches/icefaces-ee-1.8.2_p01/icefaces/component/src/com/icesoft/faces/component/ext/renderkit/GroupRenderer.java Modified : /icefaces-ee/branches/icefaces-ee-1.8.2_p01/icefaces/component/src/com/icesoft/faces/component/panelpopup/PanelPopupRenderer.java
        Hide
        Isuru Perera added a comment -

        Hi Yip Ng,

        I just watched the video of your testing. It works perfectly. Thank you very much for fixing the issue!

        I'm now eagerly waiting to get the fix from public SVN.

        Show
        Isuru Perera added a comment - Hi Yip Ng, I just watched the video of your testing. It works perfectly. Thank you very much for fixing the issue! I'm now eagerly waiting to get the fix from public SVN.
        Hide
        Joanne Bai added a comment -

        Verified successfully on ICEfaces-ee-1.8.2_p01 revision 21058 using Tomcat 6.0.26 + FF3.6 and IE6,7,8

        Test app is committed to repo\qa\trunk\Regression\ICE-5520

        Show
        Joanne Bai added a comment - Verified successfully on ICEfaces-ee-1.8.2_p01 revision 21058 using Tomcat 6.0.26 + FF3.6 and IE6,7,8 Test app is committed to repo\qa\trunk\Regression\ ICE-5520
        Hide
        Isuru Perera added a comment -

        Hi,

        I got the files and tested, but the fix didn't work for me.

        I used the above mentioned markup with the component showcase. When I click on Show button, the popup is displayed in the center. When I scroll the page, the popup is centered again. i.e. it always adjusts the position.

        If I drag the popup, then the auto centering stops and behaves as the video above.

        The fix didn't work in my application also. I use rendered attribute with my own popups. That's the only difference. I use rendered attribute as I have more than one modal popups in my page. (See this http://www.icefaces.org/JForum/posts/list/16121.page#60784 for the reason to use rendered attribute)

        My popups are similar to following markup

        <ice:panelPopup id="modalPnlPop"
        draggable="true" modal="true"
        positionOnLoadOnly="true"
        visible="#

        {popup.modalRendered}"
        rendered="#{popup.modalRendered}

        "
        autoCentre="true"
        clientOnly="false"
        styleClass="corePopup">

        I'm using Firefox 3.6.2 and cleared entire cache before testing.

        Show
        Isuru Perera added a comment - Hi, I got the files and tested, but the fix didn't work for me. I used the above mentioned markup with the component showcase. When I click on Show button, the popup is displayed in the center. When I scroll the page, the popup is centered again. i.e. it always adjusts the position. If I drag the popup, then the auto centering stops and behaves as the video above. The fix didn't work in my application also. I use rendered attribute with my own popups. That's the only difference. I use rendered attribute as I have more than one modal popups in my page. (See this http://www.icefaces.org/JForum/posts/list/16121.page#60784 for the reason to use rendered attribute) My popups are similar to following markup <ice:panelPopup id="modalPnlPop" draggable="true" modal="true" positionOnLoadOnly="true" visible="# {popup.modalRendered}" rendered="#{popup.modalRendered} " autoCentre="true" clientOnly="false" styleClass="corePopup"> I'm using Firefox 3.6.2 and cleared entire cache before testing.
        Hide
        Isuru Perera added a comment -

        Hi,

        I'm sorry, your video works as I have described in the previous comment. i.e. it auto centers till it is dragged.

        I was confused as it was not what I expected from the fix.

        I will explain what I wanted in another comment.

        Show
        Isuru Perera added a comment - Hi, I'm sorry, your video works as I have described in the previous comment. i.e. it auto centers till it is dragged. I was confused as it was not what I expected from the fix. I will explain what I wanted in another comment.
        Hide
        Isuru Perera added a comment -

        Hi Yip Ng,

        I'm sorry for the confusion.

        I will explain my requirement.

        I use popups similar to following markup (same as I mentioned above)

        <ice:panelPopup id="modalPnlPop"
        draggable="true" modal="true"
        positionOnLoadOnly="true"
        visible="#

        {popup.modalRendered}"
        rendered="#{popup.modalRendered}

        "
        autoCentre="true"
        clientOnly="false"
        styleClass="corePopup">

        Here I expect the popup to be auto centered only load. It must also be centered within the view port, not relative to the whole page.

        For example in the component showcase, if I click on "Show" button, then the popup should be displayed and it should position itself to the center of the view port. Then if I scroll the page, I don't want the popup to change its position. This is what I expect from positionOnLoadOnly attribute.

        My initial problem was that the popup positioning is not smooth when scrolling. Therefore I wanted to center the popup only on load and it will not change the position when page is scrolled or window is resized. That's why I used positionOnLoadOnly attribute.

        I hope you understood what I'm trying to say.

        Documentation for positionOnLoadOnly attribute says: "Centre or position popup once on load only. Any drag and drop will disable autoCentre and autoPosition."

        Your change fixed what positionOnLoadOnly attribute should be doing according to the documentation.

        But I think currently my requirement cannot be acheived with above markup.

        Show
        Isuru Perera added a comment - Hi Yip Ng, I'm sorry for the confusion. I will explain my requirement. I use popups similar to following markup (same as I mentioned above) <ice:panelPopup id="modalPnlPop" draggable="true" modal="true" positionOnLoadOnly="true" visible="# {popup.modalRendered}" rendered="#{popup.modalRendered} " autoCentre="true" clientOnly="false" styleClass="corePopup"> Here I expect the popup to be auto centered only load. It must also be centered within the view port, not relative to the whole page. For example in the component showcase, if I click on "Show" button, then the popup should be displayed and it should position itself to the center of the view port. Then if I scroll the page, I don't want the popup to change its position. This is what I expect from positionOnLoadOnly attribute. My initial problem was that the popup positioning is not smooth when scrolling. Therefore I wanted to center the popup only on load and it will not change the position when page is scrolled or window is resized. That's why I used positionOnLoadOnly attribute. I hope you understood what I'm trying to say. Documentation for positionOnLoadOnly attribute says: "Centre or position popup once on load only. Any drag and drop will disable autoCentre and autoPosition." Your change fixed what positionOnLoadOnly attribute should be doing according to the documentation. But I think currently my requirement cannot be acheived with above markup.
        Hide
        yip.ng added a comment -

        Hi Isuru,

        I still don't fully understand your requirement. Do you mean you don't want to center the dialog anymore after it first appears, even when it hasn't been dragged at all afterwards?

        Then the rendered attribute is not the cause of the problem?

        Show
        yip.ng added a comment - Hi Isuru, I still don't fully understand your requirement. Do you mean you don't want to center the dialog anymore after it first appears, even when it hasn't been dragged at all afterwards? Then the rendered attribute is not the cause of the problem?
        Hide
        Isuru Perera added a comment -

        Hi Yip Ng,

        Thank you very much for the reply.

        Yes, what you said is what I wanted. I expect the popup to be centered only when it is shown. Then I don't want to change its position afterward.

        And yes, "rendered" attribute is not an issue.

        Show
        Isuru Perera added a comment - Hi Yip Ng, Thank you very much for the reply. Yes, what you said is what I wanted. I expect the popup to be centered only when it is shown. Then I don't want to change its position afterward. And yes, "rendered" attribute is not an issue.
        Hide
        Isuru Perera added a comment -

        Hi Yip Ng,

        One reason that I didn't want the popup to be auto centered is that the positioning code is not smooth. For example in Firefox, when I scroll I can see the popup jerks when adjusting the position. It works well in Safari and Chrome.

        Therefore as a workaround, I wanted to display the popup in center and stop auto centering after appearance.

        For example see the popups in http://ckeditor.com/demo. I expected that behavior in ICEfaces popups.

        I hope you understand what I'm trying to say.

        Anyway, I am happy with what I have now. ICEfaces is always a great product!

        Thank you for the help!

        Show
        Isuru Perera added a comment - Hi Yip Ng, One reason that I didn't want the popup to be auto centered is that the positioning code is not smooth. For example in Firefox, when I scroll I can see the popup jerks when adjusting the position. It works well in Safari and Chrome. Therefore as a workaround, I wanted to display the popup in center and stop auto centering after appearance. For example see the popups in http://ckeditor.com/demo . I expected that behavior in ICEfaces popups. I hope you understand what I'm trying to say. Anyway, I am happy with what I have now. ICEfaces is always a great product! Thank you for the help!
        Hide
        yip.ng added a comment -

        Hi Isuru,

        Unfortunately, after some consideration, we have decided to keep the current behavior for now.

        The popup jerks are barely discernible to me. Maybe it is browser and machine dependent.

        The CKEditor dialog uses fixed positioning. It's fixed with reference to the top left corner of the window, and it can look very off center when resizing the window to be smaller or larger. Our approach is totally different. It's more dynamic, sophisticated and flexible.

        Maybe you can try using a fixed-positioned div (or <ice:panelGroup>) to look like what's in CKEditor? (In this case you probably have to layout the dialog content yourself, not just put a <ice:panelPopup> in it, because the JavaScript will still try to position the <ice:panelPopup> itself.)

        Show
        yip.ng added a comment - Hi Isuru, Unfortunately, after some consideration, we have decided to keep the current behavior for now. The popup jerks are barely discernible to me. Maybe it is browser and machine dependent. The CKEditor dialog uses fixed positioning. It's fixed with reference to the top left corner of the window, and it can look very off center when resizing the window to be smaller or larger. Our approach is totally different. It's more dynamic, sophisticated and flexible. Maybe you can try using a fixed-positioned div (or <ice:panelGroup>) to look like what's in CKEditor? (In this case you probably have to layout the dialog content yourself, not just put a <ice:panelPopup> in it, because the JavaScript will still try to position the <ice:panelPopup> itself.)
        Hide
        Isuru Perera added a comment -

        Hi Yip Ng,

        I accept the current behavior of the popups and thank you very much for the suggestion.

        I also noticed that CKEditor uses fixed positioning and that's why it's more stable. ICEfaces auto centering is little bit slow on Firefox. It works really well in Chrome and Opera. I think the slowness is because of the calculation of the position on each event.

        Anyway, I decided to use draggable="true", modal="true", positionOnLoadOnly="true" and autoCentre="true" with my popups.

        I got another issue with the fix. When the popup is first displayed, it auto centers when scrolling and resizing. After dragging it stops.

        When I close the popup and open it again, auto centering doesn't work.

        What I noticed is that the dragged property is always true after it has been dragged. This can be reproduced from the component showcase.

        Please try following steps.
        1. Open the popup.
        2. Resize or scroll. It auto centers
        3. Drag the popup.
        4. Close it.
        5. Open again. The popup opens in the center.
        6. Resize or scroll. It does not auto center.

        Please let me know whether you can also reproduce this.

        Thanks.

        Show
        Isuru Perera added a comment - Hi Yip Ng, I accept the current behavior of the popups and thank you very much for the suggestion. I also noticed that CKEditor uses fixed positioning and that's why it's more stable. ICEfaces auto centering is little bit slow on Firefox. It works really well in Chrome and Opera. I think the slowness is because of the calculation of the position on each event. Anyway, I decided to use draggable="true", modal="true", positionOnLoadOnly="true" and autoCentre="true" with my popups. I got another issue with the fix. When the popup is first displayed, it auto centers when scrolling and resizing. After dragging it stops. When I close the popup and open it again, auto centering doesn't work. What I noticed is that the dragged property is always true after it has been dragged. This can be reproduced from the component showcase. Please try following steps. 1. Open the popup. 2. Resize or scroll. It auto centers 3. Drag the popup. 4. Close it. 5. Open again. The popup opens in the center. 6. Resize or scroll. It does not auto center. Please let me know whether you can also reproduce this. Thanks.
        Hide
        Ken Fyten added a comment -

        Hi Isuru,

        This is actually the expected behavior. In this configuration, if the user drags the popup, then it is disables the autocenter setting in order to preserve their positioning of the popup. If you would like to restore the autocentering using the same component instance after it has been hidden and reshown, you could add a value-binding to the autocenter attribute and reset it to true when the popup is shown again.

        Ken

        Show
        Ken Fyten added a comment - Hi Isuru, This is actually the expected behavior. In this configuration, if the user drags the popup, then it is disables the autocenter setting in order to preserve their positioning of the popup. If you would like to restore the autocentering using the same component instance after it has been hidden and reshown, you could add a value-binding to the autocenter attribute and reset it to true when the popup is shown again. Ken
        Hide
        Isuru Perera added a comment -

        Hi Ken,

        Thanks for the reply..

        So, that's again the expected behavior. For me it was unexpected.

        I thought closing popup should reset the earlier dragged state.

        Anyway, thanks for letting me know. I think what I need to reset is the dragged state. I will see.

        I hope I the next release is ready now!

        Thanks.

        Isuru.

        Show
        Isuru Perera added a comment - Hi Ken, Thanks for the reply.. So, that's again the expected behavior. For me it was unexpected. I thought closing popup should reset the earlier dragged state. Anyway, thanks for letting me know. I think what I need to reset is the dragged state. I will see. I hope I the next release is ready now! Thanks. Isuru.

          People

          • Assignee:
            Joanne Bai
            Reporter:
            yip.ng
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: