ICEfaces
  1. ICEfaces
  2. ICE-8774

Optimize onElementUpdate implementation

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2
    • Fix Version/s: 3.3
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      ICEfaces
    • Assignee Priority:
      P2

      Description


      Many components make use of complex JavaScript in addition to HTML and CSS. The use of this JavaScript and its initialization and cleanup should be optimized, particularly for components that are used repeatedly in a datatable.

        Activity

        Hide
        Ted Goddard added a comment - - edited

        The current cleanup code contains the following which Mark has observed has an unnecessary var element = document.getElementById(id). This has not been removed, however, since it is possible that idCallbackTuple.handler assumes the existence of the element:
        elementUpdateListeners = reject(elementUpdateListeners, function(idCallbackTuple) {
        var id = idCallbackTuple.identifier;
        var element = document.getElementById(id);
        //test if inner element still exists, sometimes client side code can remove DOM fragments
        if (element) {
        var updated = (-1 !=
        idCallbackTuple.ancestors.indexOf(" " + updatedElementId + " "));
        if (updated) {
        var callback = idCallbackTuple.handler;

        Show
        Ted Goddard added a comment - - edited The current cleanup code contains the following which Mark has observed has an unnecessary var element = document.getElementById(id). This has not been removed, however, since it is possible that idCallbackTuple.handler assumes the existence of the element: elementUpdateListeners = reject(elementUpdateListeners, function(idCallbackTuple) { var id = idCallbackTuple.identifier; var element = document.getElementById(id); //test if inner element still exists, sometimes client side code can remove DOM fragments if (element) { var updated = (-1 != idCallbackTuple.ancestors.indexOf(" " + updatedElementId + " ")); if (updated) { var callback = idCallbackTuple.handler;
        Hide
        Ken Fyten added a comment -

        Assign to Mircea to apply the noted improvement.

        Show
        Ken Fyten added a comment - Assign to Mircea to apply the noted improvement.
        Hide
        Mark Collette added a comment -

        var id = idCallbackTuple.identifier;
        var element = lookupElementById(id);
        if (element) {
            var updated = contains(idCallbackTuple.ancestors, updatedElementId);
            if (updated) {
                var callback = idCallbackTuple.handler;
                try {
                   callback(element);

        If the ancestors contains check is faster than lookupElementById, then we should swap the order. And if we do get the element, we should pass that into the callback, so that the callback functions won't have to immediately find the element themselves for their cleanup.

        Show
        Mark Collette added a comment - var id = idCallbackTuple.identifier; var element = lookupElementById(id); if (element) {     var updated = contains(idCallbackTuple.ancestors, updatedElementId);     if (updated) {         var callback = idCallbackTuple.handler;         try {            callback(element); If the ancestors contains check is faster than lookupElementById, then we should swap the order. And if we do get the element, we should pass that into the callback, so that the callback functions won't have to immediately find the element themselves for their cleanup.
        Hide
        Mircea Toma added a comment -

        Reversed test order to make findAndNotifyUpdatedElement function more efficient.

        Show
        Mircea Toma added a comment - Reversed test order to make findAndNotifyUpdatedElement function more efficient.

          People

          • Assignee:
            Mircea Toma
            Reporter:
            Ted Goddard
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: