ICEfaces
  1. ICEfaces
  2. ICE-7681

ace:accordion with panechange event used with ace:ajax - not receiving events if dynamic attribute is enabled

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0
    • Fix Version/s: 3.2
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      ICEfaces3.0.0. bundle1 (revision 27312)
      Browsers: all
      Server: Tomcat6
    • Affects:
      Compatibility/Configuration

      Description

      The ace:accordion when used with the 'panechange' event of the ace:ajax component - does not register events in the same way as it does when used with the paneChangeListener attribute enabled. This happens in all browsers when either the dynamic or the collapsible attributes of the ace:accordion is set to true.

      If dynamic=true, when accordion changes panes, the ajax paneChange event is not received. This is different for an accordion with a paneChangeListener, that will always receive a pane change event, also when dynamic=true.

      If collapsible=true, when collapsing the accordion's open pane followed by its re-opening, an accordion with an ace:ajax component using its paneChange event will receive 2 events, while an accordion with a paneChangeListener will register only one event.

      This can be reproduced with the accordion test application located at:
      http://server.ice:8888/svn/repo/qa/trunk/Regression-Icefaces2/Sparkle/Nightly/accordion
      - load the attribute page: http://localhost:8080/accordion/accordionAttribute.jsf
      - change accordion panes and notice the AjaxBehaviorEvent messages with dynamic attribute unchecked, then with dynamic attribute enabled.
      - enable collapsible=true and notice the AjaxBehaviorEvent messages.

        Issue Links

          Activity

          Hide
          Arturo Zambrano added a comment -

          Here are some remarks about the feasibility of solving this issue.

          When dynamic=true, a request to the server is made every time a pane is opened in order to load its content. To accomplish this, it is necessary to do an custom update (i.e. with the ice.customUpdate parameter) and pass an onSucess callback to the ajax request. Meanwhile, the ace ajax request is rendered as a function that must be called in the client as it is (with some optional parameters). For this reasons, it is not readily feasible to fire both the dynamic tab loading request and the ajax request. We could fire both events, but that would require two requests to be made. This would be easier if instead of rendering a function for ace ajax, we rendered instead a JSON object with all the data needed to make the ajax request. This object could be manipulated in the client side to satisfy different scenarios.

          When collapsible=true, the paneChangeListener method binding is not invoked because the active index is now -1, and there's no value in the '_newTab' parameter, so a pane object can't be resolved and sent to the AccordionPaneChangeEvent event. This is more a matter of deciding whether we want to consider collapsing all panes as a pane change event or not. It would be easier to consider this a separate ajax event, let's say 'allPanesCollapsed'.

          Show
          Arturo Zambrano added a comment - Here are some remarks about the feasibility of solving this issue. When dynamic=true, a request to the server is made every time a pane is opened in order to load its content. To accomplish this, it is necessary to do an custom update (i.e. with the ice.customUpdate parameter) and pass an onSucess callback to the ajax request. Meanwhile, the ace ajax request is rendered as a function that must be called in the client as it is (with some optional parameters). For this reasons, it is not readily feasible to fire both the dynamic tab loading request and the ajax request. We could fire both events, but that would require two requests to be made. This would be easier if instead of rendering a function for ace ajax, we rendered instead a JSON object with all the data needed to make the ajax request. This object could be manipulated in the client side to satisfy different scenarios. When collapsible=true, the paneChangeListener method binding is not invoked because the active index is now -1, and there's no value in the '_newTab' parameter, so a pane object can't be resolved and sent to the AccordionPaneChangeEvent event. This is more a matter of deciding whether we want to consider collapsing all panes as a pane change event or not. It would be easier to consider this a separate ajax event, let's say 'allPanesCollapsed'.
          Hide
          Arturo Zambrano added a comment -

          This is a complete analysis of all the possible options to solve this JIRA.

          There are two distinct issues in this JIRA that can be summarized as follows:
          1. When collapsible=true, the paneChangeListener is not fired when collapsing the active pane, but the ace:ajax event is.
          2. When dynamic=true, ace:ajax events aren't fired when changing panes, but the paneChangeListener is.

          For the first issue, it is a matter of deciding how we want the component to behave. There are three options:
          1. Do not fire the ace:ajax event, when collapsible=true and all panes are collapsed.
          2. Do fire the paneChangeListener, but set the pane argument to null when all panes are collapsed and document this behaviour.
          3. Define a separate event to fire when all panes are collapsed and create a new ace:ajax event and a new listener attribute for this event.

          The second issue is more complex and cannot be easily fixed. The reason is that the dynamic pane-loading request and the ajax request are fundamentally different, and their operations cannot be combined in a single request. When using dynamic loading, a request is sent to the server with the 'ice.customUpdate' parameter and a callback is passed to be executed when the response arrives. In the server side, the component simply renders the content of the pane to be loaded – instead of its original markup. Since this is a custom update, back in the client, the callback that was passed takes the response body and puts it as the content of the active pane. Therefore, an ace:ajax event that tries to be processed in the same request would cause problems, especially if rendering @all, @form, @this, or any children of the accordion component, because it would be necessary to render the accordion component normally in order to correctly determine what needs to be updated in the client, and as mentioned before, the normal rendering of the component is manipulated when using the dynamic loading functionality.
          These are our options:
          1. Make two separate requests, instead of just one: one to load the pane dynamically and another for the ajax event.
          2. Remove the dynamic loading functionality altogether and let the app developer control when the panes are loaded/rendered, by using 'rendered' attributes for each of the panes and using ace:ajax events to specify when to load/update the panes.
          3. Keep the dynamic loading functionality and simply document that ace:ajax events cannot be fired when using dynamic loading.
          4. Modify the dynamic loading functionality to work in a more "natural" way. Basically, it would consist of avoiding all this custom handling, and simply render the component normally without rendering the contents of all panes, but only the contents of the active pane. This might not have the exact same effect as the current dynamic loading functionality, as the whole component would be rendered this time (i.e. the component's skeleton markup and script, which would be executed again).

          Show
          Arturo Zambrano added a comment - This is a complete analysis of all the possible options to solve this JIRA. There are two distinct issues in this JIRA that can be summarized as follows: 1. When collapsible=true, the paneChangeListener is not fired when collapsing the active pane, but the ace:ajax event is. 2. When dynamic=true, ace:ajax events aren't fired when changing panes, but the paneChangeListener is. For the first issue, it is a matter of deciding how we want the component to behave. There are three options: 1. Do not fire the ace:ajax event, when collapsible=true and all panes are collapsed. 2. Do fire the paneChangeListener, but set the pane argument to null when all panes are collapsed and document this behaviour. 3. Define a separate event to fire when all panes are collapsed and create a new ace:ajax event and a new listener attribute for this event. The second issue is more complex and cannot be easily fixed. The reason is that the dynamic pane-loading request and the ajax request are fundamentally different, and their operations cannot be combined in a single request. When using dynamic loading, a request is sent to the server with the 'ice.customUpdate' parameter and a callback is passed to be executed when the response arrives. In the server side, the component simply renders the content of the pane to be loaded – instead of its original markup. Since this is a custom update, back in the client, the callback that was passed takes the response body and puts it as the content of the active pane. Therefore, an ace:ajax event that tries to be processed in the same request would cause problems, especially if rendering @all, @form, @this, or any children of the accordion component, because it would be necessary to render the accordion component normally in order to correctly determine what needs to be updated in the client, and as mentioned before, the normal rendering of the component is manipulated when using the dynamic loading functionality. These are our options: 1. Make two separate requests, instead of just one: one to load the pane dynamically and another for the ajax event. 2. Remove the dynamic loading functionality altogether and let the app developer control when the panes are loaded/rendered, by using 'rendered' attributes for each of the panes and using ace:ajax events to specify when to load/update the panes. 3. Keep the dynamic loading functionality and simply document that ace:ajax events cannot be fired when using dynamic loading. 4. Modify the dynamic loading functionality to work in a more "natural" way. Basically, it would consist of avoiding all this custom handling, and simply render the component normally without rendering the contents of all panes, but only the contents of the active pane. This might not have the exact same effect as the current dynamic loading functionality, as the whole component would be rendered this time (i.e. the component's skeleton markup and script, which would be executed again).
          Hide
          Ken Fyten added a comment -
          For the first issue, it is a matter of deciding how we want the component to behave. There are three options:
          1. Do not fire the ace:ajax event, when collapsible=true and all panes are collapsed.
          2. Do fire the paneChangeListener, but set the pane argument to null when all panes are collapsed and document this behaviour.
          3. Define a separate event to fire when all panes are collapsed and create a new ace:ajax event and a new listener attribute for this event.

          Instead of any of the above, can we simply make it so both the ace:ajax event and the paneChangeListener only fire when a new pane is shown, not when a previously shown pane is hidden? That way there would only ever be a single event fired for each switch from one pane to another. If all panes were collapsed, no event would be fired.

          4. Modify the dynamic loading functionality to work in a more "natural" way. Basically, it would consist of avoiding all this custom handling, and simply render the component normally without rendering the contents of all panes, but only the contents of the active pane. This might not have the exact same effect as the current dynamic loading functionality, as the whole component would be rendered this time (i.e. the component's skeleton markup and script, which would be executed again).

          This seems like the best option, plus it gets us very close to being able to remove support for the "customUpdate" scenario altogether in ACE.

          Show
          Ken Fyten added a comment - For the first issue, it is a matter of deciding how we want the component to behave. There are three options: 1. Do not fire the ace:ajax event, when collapsible=true and all panes are collapsed. 2. Do fire the paneChangeListener, but set the pane argument to null when all panes are collapsed and document this behaviour. 3. Define a separate event to fire when all panes are collapsed and create a new ace:ajax event and a new listener attribute for this event. Instead of any of the above, can we simply make it so both the ace:ajax event and the paneChangeListener only fire when a new pane is shown, not when a previously shown pane is hidden? That way there would only ever be a single event fired for each switch from one pane to another. If all panes were collapsed, no event would be fired. 4. Modify the dynamic loading functionality to work in a more "natural" way. Basically, it would consist of avoiding all this custom handling, and simply render the component normally without rendering the contents of all panes, but only the contents of the active pane. This might not have the exact same effect as the current dynamic loading functionality, as the whole component would be rendered this time (i.e. the component's skeleton markup and script, which would be executed again). This seems like the best option, plus it gets us very close to being able to remove support for the "customUpdate" scenario altogether in ACE.
          Hide
          Carmen Cristurean added a comment - - edited

          This fix seems to cause a regression to the accordion attribute page with the paneChangeListener attribute enabled (http://localhost:8080/accordion/accordionAttribute.jsf), the activeIndex on this page is not updated on the page anymore when changing panes.
          Wondering if this fix requires any code changes for this test?
          Also, the accordionAjax -> PaneChangeListener attribute only test started failing (all browsers):
          Expected message = "AjaxBehaviorEvent received! (#1)", Actual message =
          Expected value A = 1, Actual value =0
          Expected value B = 1, Actual value =0
          Is this correct?

          Show
          Carmen Cristurean added a comment - - edited This fix seems to cause a regression to the accordion attribute page with the paneChangeListener attribute enabled ( http://localhost:8080/accordion/accordionAttribute.jsf ), the activeIndex on this page is not updated on the page anymore when changing panes. Wondering if this fix requires any code changes for this test? Also, the accordionAjax -> PaneChangeListener attribute only test started failing (all browsers): Expected message = "AjaxBehaviorEvent received! (#1)", Actual message = Expected value A = 1, Actual value =0 Expected value B = 1, Actual value =0 Is this correct?
          Hide
          Arturo Zambrano added a comment - - edited

          Actually, I forgot to mention that there's a change in behaviour. Now, collapsing all the panes should not trigger a paneChange event either via ace:ajax or the paneChangeListener attribute, as described in Ken's comment above.
          This behaviour should be the same in Mojarra and Myfaces. I get the same behaviour on my machine. Please confirm if this is the new behaviour in both frameworks.

          Show
          Arturo Zambrano added a comment - - edited Actually, I forgot to mention that there's a change in behaviour. Now, collapsing all the panes should not trigger a paneChange event either via ace:ajax or the paneChangeListener attribute, as described in Ken's comment above. This behaviour should be the same in Mojarra and Myfaces. I get the same behaviour on my machine. Please confirm if this is the new behaviour in both frameworks.
          Hide
          Carmen Cristurean added a comment - - edited

          Using rev. 31084, there is a JS error that occurs when loading the accordion attribute page in IE7 browser (either Mojarra and MyFaces).
          This was not an issue before.

          Show
          Carmen Cristurean added a comment - - edited Using rev. 31084, there is a JS error that occurs when loading the accordion attribute page in IE7 browser (either Mojarra and MyFaces). This was not an issue before.
          Hide
          Ken Fyten added a comment - - edited

          The JS error with IE7 is related to a bridge change Mircea committed this morning and is being dealt with under a different JIRA by him.
          (Restricted to icesoft-internal-developers group)

          Show
          Ken Fyten added a comment - - edited The JS error with IE7 is related to a bridge change Mircea committed this morning and is being dealt with under a different JIRA by him. (Restricted to icesoft-internal-developers group)
          Hide
          Arturo Zambrano added a comment - - edited

          Committed fix to the trunk at revision 30947.
          The fix is for executing the paneChangeListener and ajax paneChange event in one request. Also the behaviour was modified so that only opening new panes triggers the paneChangeListener and ajax listener and not when closing them. The 'dynamic' and 'cache' attributes are now deprecated. Now, every tab change produces a request, and the contents of the panes will be dynamically updated if they differ from the previous content. This component no longer needs to use the 'ice.customUpdate' parameter.

          Show
          Arturo Zambrano added a comment - - edited Committed fix to the trunk at revision 30947. The fix is for executing the paneChangeListener and ajax paneChange event in one request. Also the behaviour was modified so that only opening new panes triggers the paneChangeListener and ajax listener and not when closing them. The 'dynamic' and 'cache' attributes are now deprecated. Now, every tab change produces a request, and the contents of the panes will be dynamically updated if they differ from the previous content. This component no longer needs to use the 'ice.customUpdate' parameter.

            People

            • Assignee:
              Arturo Zambrano
              Reporter:
              Carmen Cristurean
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: