ICEfaces
  1. ICEfaces
  2. ICE-10798

ace:dataTable, multiple requests fired when using the paginator via the keyboard

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.1
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      Any

      Description

      When the paginator is enabled in the data table, navigating to the next or previous pages via the PgDn and PgUp keys produces multiple requests. The number of requests increases every time these keys are pressed. Depending on the sequence of keys used and the times they are used some flickering might be visible. The flickering is clearly visible when live scrolling is enabled as well.

      The reason why this happens is that when the table is recreated it calls its destroy() method to clear the table of event listeners. The problem is that the call to remove the event listener for these keys in the paginator references the event handler of the current (new) instance and not the handler of the previous instance, so nothing gets removed and all these listeners fire at the same time.

      So, the onElementUpdate() call has to be fixed or the previous instance has to be passed to the destroy method or a static event handler should be used instead, if possible.

        Activity

        Arturo Zambrano created issue -
        Ken Fyten made changes -
        Field Original Value New Value
        Assignee Arturo Zambrano [ artzambrano ]
        Fix Version/s 4.1 [ 11375 ]
        Assignee Priority P2 [ 10011 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #45983 Thu Sep 17 14:37:30 MDT 2015 art.zambrano ICE-10798 added mechanism to retrieve the previous table instance object and call destroy() on it before recreating the new table instance
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Hide
        Arturo Zambrano added a comment -

        r45983: added mechanism to retrieve the previous table instance object and call destroy() on it before recreating the new table instance.

        This issue started after introducing client IDs to the table body and table setup script. Because of this, the onElementUpdate() element doesn't fire when only the body and/or the setup script are updated (as opposed to the entire table). As a result, the destroy() method doesn't get called in the previous instance, and many event listeners accumulate. Since adding additional onElementUpdate calls for these specific nodes didn't work, the fix consists in saving a reference to the current table instance in a global variable, and then accessing this instance when the same table is about to be recreated, invoking the destroy() method of the previous instance to remove all the previous event listeners and avoid accumulating them.

        Show
        Arturo Zambrano added a comment - r45983: added mechanism to retrieve the previous table instance object and call destroy() on it before recreating the new table instance. This issue started after introducing client IDs to the table body and table setup script. Because of this, the onElementUpdate() element doesn't fire when only the body and/or the setup script are updated (as opposed to the entire table). As a result, the destroy() method doesn't get called in the previous instance, and many event listeners accumulate. Since adding additional onElementUpdate calls for these specific nodes didn't work, the fix consists in saving a reference to the current table instance in a global variable, and then accessing this instance when the same table is about to be recreated, invoking the destroy() method of the previous instance to remove all the previous event listeners and avoid accumulating them.
        Arturo Zambrano made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Liana Munroe added a comment -

        Tested with ICEfaces 4 trunk r45986. Tomcat 7
        Performance of scroll bar using pageUp/pageDown is good while using Firefox, with or without live scrolling.
        When testing with Chrome/IE 11 using pageUp/pageDown keys the scrolling stops after a couple of pages. Clicking on the scroll bar, then pressing a page key will resume the scrolling.
        With IE 11, the pageUp/pageDown keys still seem to produce multiple requests as per the original issue in this JIRA and there is no scrolling animation in the scroll bar when scrolling with the page keys.

        Show
        Liana Munroe added a comment - Tested with ICEfaces 4 trunk r45986. Tomcat 7 Performance of scroll bar using pageUp/pageDown is good while using Firefox, with or without live scrolling. When testing with Chrome/IE 11 using pageUp/pageDown keys the scrolling stops after a couple of pages. Clicking on the scroll bar, then pressing a page key will resume the scrolling. With IE 11, the pageUp/pageDown keys still seem to produce multiple requests as per the original issue in this JIRA and there is no scrolling animation in the scroll bar when scrolling with the page keys.
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Assignee Priority P2 [ 10011 ] P1 [ 10010 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #46060 Thu Oct 01 11:34:20 MDT 2015 art.zambrano ICE-10798 fix for returning focus to table body after pressing PgDn, PgUp, Home, and End and thus allow to continue using keyboard navigation; added timeout flag to avoid processing scrolling events a fraction of a second after setting up the scrolling events, in order to avoid immediate subsequent requests on IE
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/paginator/paginator.js
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/src/org/icefaces/ace/component/datatable/DataTableRenderer.java
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.css
        Hide
        Arturo Zambrano added a comment -

        r46060: fix for returning focus to table body after pressing PgDn, PgUp, Home, and End and thus allow to continue using keyboard navigation; added timeout flag to avoid processing scrolling events a fraction of a second after setting up the scrolling events, in order to avoid immediate subsequent requests on IE.

        Show
        Arturo Zambrano added a comment - r46060: fix for returning focus to table body after pressing PgDn, PgUp, Home, and End and thus allow to continue using keyboard navigation; added timeout flag to avoid processing scrolling events a fraction of a second after setting up the scrolling events, in order to avoid immediate subsequent requests on IE.
        Arturo Zambrano made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Liana Munroe added a comment -

        Tested with ICEfaces 4 trunk r46060, Tomcat 7 and QA Live Scrolling Test apps in the dataTable test suite. Using PgUp / PgDn buttons seems to have different effects depending on the browser used.
        Chrome 45 - If paginator is on page 1, using the PgUp / PgDn buttons will scroll up and down the scroll bar but stops at the top or bottom and no further pages are scrolled to. The paginator stays on page 1. If you change the paginator page to a higher number, then use the PgUp / PgDn buttons the paginator gets scrolled through quickly. It will scroll several pages if you hold down the button for a moment. It will scroll in the intended direction, then back the other way for a few pages.
        FF - When first loading an app in FF the PgUp / PgDn buttons do not appear to function. If you select a paginator button higher than 1 then use the PgUp / PgDn buttons the page will scroll but much in the manner of Chrome also seems to roam up and down the paginator.
        IE 11 - Slight paginator roaming as above.

        Show
        Liana Munroe added a comment - Tested with ICEfaces 4 trunk r46060, Tomcat 7 and QA Live Scrolling Test apps in the dataTable test suite. Using PgUp / PgDn buttons seems to have different effects depending on the browser used. Chrome 45 - If paginator is on page 1, using the PgUp / PgDn buttons will scroll up and down the scroll bar but stops at the top or bottom and no further pages are scrolled to. The paginator stays on page 1. If you change the paginator page to a higher number, then use the PgUp / PgDn buttons the paginator gets scrolled through quickly. It will scroll several pages if you hold down the button for a moment. It will scroll in the intended direction, then back the other way for a few pages. FF - When first loading an app in FF the PgUp / PgDn buttons do not appear to function. If you select a paginator button higher than 1 then use the PgUp / PgDn buttons the page will scroll but much in the manner of Chrome also seems to roam up and down the paginator. IE 11 - Slight paginator roaming as above.
        Liana Munroe made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #46062 Thu Oct 01 15:32:04 MDT 2015 art.zambrano ICE-10798 added timeout flag to avoid multiple frequent requests when holding the PgDn and PgUp (and arrows) keys
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/paginator/paginator.js
        Hide
        Arturo Zambrano added a comment -

        r46062: added timeout flag to avoid multiple frequent requests when holding the PgDn and PgUp (and arrows) keys

        Show
        Arturo Zambrano added a comment - r46062: added timeout flag to avoid multiple frequent requests when holding the PgDn and PgUp (and arrows) keys
        Hide
        Ken Fyten added a comment -

        To clarify, when the paginator is present, and vertical scrolling is not enabled, then PgUp/PgDwn/Home/End when pressed in the body of the dataTable should paginate and go to the first/last page.

        However, when the paginator is present, and vertical scrolling is also enabled, pressing PgUp/PgDwn, etc. in the body of the dataTable should instead apply to the vertical scrollbar position. Only if focus is inside the paginator bar itself should the PgUp/PgDwn keys cause pagination directly, instead of moving the scrollbar.

        Show
        Ken Fyten added a comment - To clarify, when the paginator is present, and vertical scrolling is not enabled, then PgUp/PgDwn/Home/End when pressed in the body of the dataTable should paginate and go to the first/last page. However, when the paginator is present, and vertical scrolling is also enabled, pressing PgUp/PgDwn, etc. in the body of the dataTable should instead apply to the vertical scrollbar position. Only if focus is inside the paginator bar itself should the PgUp/PgDwn keys cause pagination directly, instead of moving the scrollbar.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #46065 Fri Oct 02 13:34:48 MDT 2015 art.zambrano ICE-10798 modified paginator keyboard navigation to only be triggered when the paginator panel(s) or paginator controls have focus if there's a vertical scrollbar, allowing for the default behaviour on the scrollbar, otherwise, the keyboard navigation is triggered on the entire table
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/paginator/paginator.js
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/src/org/icefaces/ace/component/datatable/DataTableRenderer.java
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.css
        Hide
        Arturo Zambrano added a comment -

        r46065: modified paginator keyboard navigation to only be triggered when the paginator panel(s) or paginator controls have focus if there's a vertical scrollbar, allowing for the default behaviour on the scrollbar, otherwise, the keyboard navigation is triggered on the entire table.

        Show
        Arturo Zambrano added a comment - r46065: modified paginator keyboard navigation to only be triggered when the paginator panel(s) or paginator controls have focus if there's a vertical scrollbar, allowing for the default behaviour on the scrollbar, otherwise, the keyboard navigation is triggered on the entire table.
        Arturo Zambrano made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Ken Fyten added a comment -

        Okay, testing now indicates that the PgUp/Dn, etc. keys work as expected when focus is either in the table body, or on one of the paginator buttons.

        However, live-scrolling is not working at all anymore, moving the scrollbar to the bottom or top doesn't trigger a pagination.

        Also on FF and Chrome, if the scrollbar slider or buttons are clicked (focussed), the PgUp/Dn keys do not move the scrollbar. This does work on IE however, and would make sense if it worked on all browsers if possible.

        Show
        Ken Fyten added a comment - Okay, testing now indicates that the PgUp/Dn, etc. keys work as expected when focus is either in the table body, or on one of the paginator buttons. However, live-scrolling is not working at all anymore, moving the scrollbar to the bottom or top doesn't trigger a pagination. Also on FF and Chrome, if the scrollbar slider or buttons are clicked (focussed), the PgUp/Dn keys do not move the scrollbar. This does work on IE however, and would make sense if it worked on all browsers if possible.
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #46070 Tue Oct 06 10:13:36 MDT 2015 art.zambrano ICE-10798 removed previous fix that was now interfering with livescrolling and is no longer necessary
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/datatable/datatable.js
        Hide
        Arturo Zambrano added a comment -

        r46070: removed previous fix that was now interfering with livescrolling and is no longer necessary.

        Show
        Arturo Zambrano added a comment - r46070: removed previous fix that was now interfering with livescrolling and is no longer necessary.
        Arturo Zambrano made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Liana Munroe added a comment -

        Verified ICEfaces 4 trunk r46073. Tomcat 7, IE 11, 10, 9, 8, FF 34, Chrome 45.

        Show
        Liana Munroe added a comment - Verified ICEfaces 4 trunk r46073. Tomcat 7, IE 11, 10, 9, 8, FF 34, Chrome 45.
        Hide
        Liana Munroe added a comment -

        Reopening because the arrow keys are not functioning correctly in ace:dataTable when filtering is used and scrollable is false. Please see forum post: http://jforum.icesoft.org/JForum/posts/list/23007.page#80463.
        This can be demonstrated with the showcase ace:dataTable >Overview demo. Attempting to use arrow keys in a filter field fails. After adding scrollable = "true " to this demo, the failure no longer occurs.

        Show
        Liana Munroe added a comment - Reopening because the arrow keys are not functioning correctly in ace:dataTable when filtering is used and scrollable is false. Please see forum post: http://jforum.icesoft.org/JForum/posts/list/23007.page#80463 . This can be demonstrated with the showcase ace:dataTable >Overview demo. Attempting to use arrow keys in a filter field fails. After adding scrollable = "true " to this demo, the failure no longer occurs.
        Liana Munroe made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Ken Fyten made changes -
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #46141 Tue Oct 27 15:29:00 MDT 2015 art.zambrano ICE-10798 fix to ignore paginator keyboard events when filter fields have focus, thus allowing the use of keyboard arrow keys in filter fields
        Files Changed
        Commit graph MODIFY /icefaces4/trunk/icefaces/ace/component/resources/icefaces.ace/paginator/paginator.js
        Hide
        Arturo Zambrano added a comment -

        r46141: fix to ignore paginator keyboard events when filter fields have focus, thus allowing the use of keyboard arrow keys in filter fields.

        Show
        Arturo Zambrano added a comment - r46141: fix to ignore paginator keyboard events when filter fields have focus, thus allowing the use of keyboard arrow keys in filter fields.
        Arturo Zambrano made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Carmen Cristurean added a comment -

        Verified with ICEfaces4 trunk r46143 / IE11, FF34, Chrome46.

        Show
        Carmen Cristurean added a comment - Verified with ICEfaces4 trunk r46143 / IE11, FF34, Chrome46.
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Arturo Zambrano
            Reporter:
            Arturo Zambrano
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: