Monday, June 29, 2015

SlickGrid Async Post Render Cleanup

This SlickGrid issue has been a bit of a hot potato.

Discussions at:
  github.com/mleibman/SlickGrid/issues/82
  github.com/mleibman/SlickGrid/issues/855
in particular.

So MLeibman has gone on record as saying that using jQuery data and event handlers in AsyncPostRender nodes is not a good idea and that allowing a cleanup might give people the idea that it's OK, which it really isn't.
The counter argument run along the lines that jQuery is used more and more for event and data binding, and that the amount of binding is a continuum - it may be used only lightly, all the way to fully fledged jQueryUI plugins, thus there may be a valid use case, and if so, having provided the means to implement it the grid should provide the means to clean up.

Having spent some time maintaining the SubSonic micro-ORM, one of the things I liked most about the project was the philosophy that the developer is in the end responsible for their decisions, and it is the place of the framework developer to simply offer as wide a range of options as possible - if necessary, also opening up the possibility of these options being abused.
So in this spirit, I decided to try to work out a flexible cleanup method for async post rendered nodes that would also work asynchronously, just like the post render method itself.

After examination of the code, and an initial attempt to code a solution, it became clear that there were three cleanup scenarios needing handling (remembering that one or more cells in any row might have been post rendered):
  1. deletion of an entire row from the DOM and grid cache
  2. deletion of a single cell from the DOM and grid cache
  3. the re-rendering of a previously rendered cell without cell deletion
In addition, the SlickGrid code has a workaround for a Mac scrolling issue that delays deletion of the grid node where a mouse scroll started (using var zombieRowNodeFromLastMouseWheelEvent). This is a row cleanup like the regular one, but it is handled in a different section of code and might easily escape notice. I'd just about bet that was what was causing Jonozzz's small remaining memory leak in Issue 82.

The first step was to add the example example10a-async-post-render-cleanup.html to the examples folder of my alternative master repository.

The modifications to SlickGrid added two new options to the Grid:
    enableAsyncPostRenderCleanup (bool)
    asyncPostRenderCleanupDelay (int)
and an asyncPostRenderCleanup (function) option to each column definition.
A cleanupBeforeRender parameter was added to the end of the AsyncPostRender function arguments.

Internally, where nodes were cleaned up using direct DOM manipulation previously, the code now checks to see if post render cleanup is enabled, and if the row or cell had been post rendered. If so, any post rendered cells are added to a cleanup queue, followed by the row node in the case of row deletion.
Any process that generates queue events afterwards kicks off the post render cleanup on timed delay.
A cleanup queue push example:
postProcessedCleanupQueue.push({
    actionType: 'C',
    groupId: postProcessgroupId,
    node: cacheEntry.cellNodesByColumnIdx[ columnIdx ],
    columnIdx: columnIdx | 0,
    rowIdx: rowIdx
  });
ActionType is C to clean up a post rendered cell, and R to delete a parent row node once the child cells have been cleaned up.GroupId groups cleanup records so we can clean up a row's worth of cells at a time (the same way they are post rendered). The cleanup function processes queue entries matching the GroupId of the first queue entry, then re-calls itself after a delay of asyncPostRenderCleanupDelay ms.
The node is the actual cell or row DOM node, and the column and row indexes may be used to reference the column or row data.

So case 1 (row deletion) is handled by adding any post-processed column nodes to the queue, then the row node.
Case 2 is handled by adding the cell to the queue if it has been post-processed.
Case 3 is handled by including a parameter cleanupBeforeRender (bool) in the PostRender call. If true, this indicates that the cell has been post-rendered already and is being re-rendered without deleting the cell, and that the render function should therefore clean up the previous actions prior to starting the new ones.

So as a summary, here are the key parts of the example page demonstration AsyncPostRender with cleanup:
var options = {
    ... ,
    enableAsyncPostRender: true,
    asyncPostRenderDelay: 50,
    enableAsyncPostRenderCleanup: true,
    asyncPostRenderCleanupDelay: 40
  };

  var columns = [
    ... ,
    {id: "chart", name: "Chart", formatter: waitingFormatter, rerenderOnResize: true,
        asyncPostRender: renderSparkline, asyncPostRenderCleanup: cleanupSparkline}
  ];

  function renderSparkline(cellNode, rowIdx, dataContext, colDef, cleanupBeforeRender) {
    ...
  }
  
  function cleanupSparkline(cellNode, rowIdx, colDef) {
    ...
  }

This provides a comprehensive API and async cleanup process, which should be able to handle cleanup gracefully, as long as the weight of the render and cleanup is not so great that it compromises the performance of the basic grid code.
I reiterate: it is up to the developer to plan, test and monitor the performance of the grid in conjunction with 'heavy' external controls under production conditions. It is easy to get into trouble. Use this tool wisely.

See the commit for details, and check out the sample page for usage notes.

Wednesday, June 3, 2015

SlickGrid with jQueryUI and Bootstrap

Recently I've taken up the mantle of maintaining an updated fork of MLeibman's fantastic SlickGrid trunk. I suppose it was inevitable that it would lead to a blog post.

There have been multiple issues posted about the jQuery Accordion and about Bootstrap 3 issues with sizing. After examination, the bootstrap issue is quite complex and I thought it was worth documenting the details of both.
The first step was to add stripped down, simple example pages for both cases to be used for testing. example-jquery-accordion.html and example-bootstrap-3-header.html are now present in the examples folder of my alternative master repository.

Accordion Formatting

The formatting of the SlickGrid header row was showing small inconsistencies in header size:








This was a minor CSS issue: it appears that in normal situations the header is formatted by the
.slick-header-column.ui-state-default class, which is being evaluated as more specific than (hence takes precedence over) the generic jQueryUI ui-state-default class.
When enclosed in the accordion div (and I'd assume tabs or any other similar container), ui-state-default gets precedence and adds extra border segments.
It is easily fixed by adding the !important tag to various SlickGrid header classes. This is exactly the kind of situation that !important is designed for.

.slick-header.ui-state-default, .slick-headerrow.ui-state-default {
  width: 100%;
  overflow: hidden;
  border-left: 0px !important;
}
.slick-header-column.ui-state-default {
  position: relative;
  display: inline-block;
  overflow: hidden;
  -o-text-overflow: ellipsis;
  text-overflow: ellipsis;
  height: 16px;
  line-height: 16px;
  margin: 0;
  padding: 4px;
  border-right: 1px solid silver;
  border-left: 0px !important;
  border-top: 0px !important;
  border-bottom: 0px !important;
  float: left;
}

Blank Grid Display in Accordion in IE8

More sinister was a problem with vanishing grid contents. I tested in IE8 but other IE versions may also be implicated.
Steps to reproduce using the accordion demo page:
1) open the page in IE 8 and scroll the first grid down a page or so.
2) switch to the second accordion, then back
The first grid should now be blank.

This is an IE rendering issue - IE resets the scrollbar and display when a div or its parent is hidden with display:none. Checking it out with the IE developer tools showed that the DOM elements still existed, but just weren't being shown. Probably because of this, all attempts to refresh the grid failed. Only destroying and recreating the grid was able to get past the blank display.

Because this workaround changes the screen UI, I have commented the code out in the demo page, but it is there if needed.
I was unable (and frankly unwilling) to find a solution for what appears to be an IE bug. If anyone finds a mechanism for refreshing the blank div, let me know and I'll bake it in to the grid.

Bootstrap 3 Column Width Issues

There have been many reports of column width issues under Bootstrap 3. It doesn't take long to find the culprit. Simply including the bootstrap.css file on your page includes this little gem:

* {
  -webkit-box-sizing: border-box;
     -moz-box-sizing: border-box;
          box-sizing: border-box;
}
*:before,
*:after {
  -webkit-box-sizing: border-box;
     -moz-box-sizing: border-box;
          box-sizing: border-box;
}

Admittedly, border-box is a much more sensible model, but this css forces border-box onto most elements on the page (some inputs are excluded).
The interesting thing is that the main (MLeibman) branch of SlickGrid, which is at jQuery 1.7, deals with this perfectly. The header height needs css tweaking, but the column widths resize and drag fine. It's only after we update jQuery that the trouble starts.

The problem is similar to the first image, but it only starts when resizing columns. The drag handle and column header size are offset by an amount equal to the padding and border widths of the column. Worse, the effect is cumulative each time the column is resized.

The reason is summarised here (thanks to JCReady for the heads up). The way jQuery handles box-sizing: border-box in relation to the .width and .outerWidth properties changed in jQuery 1.8. Before, .width essentially did a .css("width") which meant that it would return different numbers depending on the box-sizing setting. Afterwards, it returned the correct inner element size regardless of the box-sizing setting (note that it warns of the performance hit for this feature).
1.8 also allowed .outerWidth to be used as a setter.

Solution 1

A very easy (and tempting) solution is to follow in the footsteps of the css fixes above and add:

slick-header-column.ui-state-default {
   box-sizing: content-box !important;
}

This works just fine since the existing codebase was written to use the default box-sizing: content-box setting. However, it is conceivable that border-box could be needed on the header elements, particularly when using the menu and button features that are available. I resolved to rather solve the problem in the code.

Solution 2

Most of the forum fixes for the column sizing issue recommend replacing all occurences of .width with .outerWidth. This works for the  box-sizing: border-box case but manifests a mirror image problem with content-box (ie. the offset is negative instead of positive).
In order to preserve the correct operation under the old and new jQuery versions in both  box-sizing cases, it was necessary to sniff the jQuery version and provide an alternate code path.
In the end, it was only necessary to make a small adjustment to applyColumnHeaderWidths to solve the issue.
See the commit for code details.