Adding keyboard nav, tests, gulp, react 0.11.1

[bakesteve]

Cleaned this up a bit, but the main changes are:

Structure / tooling

• Added gulp, rather than makefiles (todos: less transform, and handle /docs)
• Added jshint
• Added examples folder, but really that should be merged into Docs
• Using bower for jasmine / jquery etc
• Added Test folder and a jasmine runner

Features

Grid Navigation

ExcelGrid wraps the Grid component and adds keyboard navigation support
Toyed with a number of ways of doing this, but the one we've gone for here is:

• ExcelGrid owns the state of what is selected
• Assume you start at row 0, col 0
• Click selects a cell
• Up/Down/Left/Right navigates, and focuses the cell
• Scrolling happens through native browser events (though think it may be better to change this Update README.md with doc link #9)
• Cells get tabindexes (so that the keyboard events fire) and a class added
• Shift Key allows multiple cells to be selected
• Always have 1 ActiveCell < so that we can add editors more easily at a later date

Scrolling

Partly as a way to get the grid working in react 0.11.0, which broke in the context and child components (being descriptors) see
To make this all work, reduce complexity, and tweak some extra performance, got rid of the custom scrollbars, and revert to the Divs scroll
May have missed some crucial details, but in our tests this wouks well (bar a small issue on horizontal scroll keeping the headers in track #9)
Primarily this revolved around removing the scrollbar components, and the DOMMetrics mixin
Let me know if there are areas I misunderstood / it has introduced regressions

React v0.11.1

As above, upgrading broke the DomMetrics mixin, so I chopped it out

Tests

Added tests (runnable via gulp) and a jasmine testRunner.html file
had to hack the gulp task a little so that I could get browserify to pick up requires properly. We plan to migrate to webpack anyway, so will revisit that then
The tests are split into core and browser, with anything under core getting run in node (via gulp)
We're planning on plugging Jest in to get a fake DOM and then shoudl be able to make most tests run in browser OR in node, so we can have a watch that automatically reactifys, es6ifys, jshints, and runs tests

The tests so far are mostly on the keyboard nav side, but clearly lots of areas to add specs on for the main grid

[bakesteve]

Closes #8

[bakesteve]

@andreypopp here you go
appologies its all in 1 big go, but I cleaned the commits a little

As I said - we thought this repo was dormant so may have been more radical / opinionated on tooling etc than was warranted, but you can now do gulp and launch a local site or gulp tests-run to launch the test runner (need to cleanup a few files for jshint to make the console less angry..)

Let me know your thoughts on this, both from a tooling / tests point of view, and on the react side of things for keyboard nav, and for the scrolling. We are relative newcomers to React, so may have taken some odd paths / done things in the wrong way

we've also listed what we'd like to see happen to extend this here - and be happy to collaborate on getting a roadmap pulled together

[jackofseattle]

You have the wrong repo here.

[bakesteve]

yep - as I said at the top of the PR, this wasnt really designed to be a PR back to the base fork - hence being 4 or 5 major alterations in one
But I'll clean that up

[jackofseattle]

I'm curious as to why we would need to merge excel functionality into the base grid?
Doing so breaks the single use premise of the component world and starts to create some hybrid jquery plugin landscape.

Can we not just have a second component that's not part of this that adds in the excel functionality?
Ultimately we'd end up with something much more light weight and modular.

[jackofseattle]

Personal preference maybe, but seems like a .jshintrc file would be a bit cleaner than sticking the declaration at the top of each file.

[bakesteve]

agree (apart form maybe esnext?) - was mostly just trying to reduce the jshint noise
I'd rather not have the unused / undef as false

[jackofseattle]

Sorry to hijack the PR. We are debating using this on a new project at the company and I'm always concerned about library size. Thanks for fixing the 0.11 issues though, that's super helpful.

[bakesteve]

@jackofseattle – yes, I did wonder if this wasn’t better placed as an extension
My initial attempt was closer to this, but ended up having to either fallback to DOM events/listeners too much or replacing large parts of the grid (ie rowRenderer)

The only real changes required to the Grid were to add SelectedCells and Row / Cell click events.
So maybe a better solution is to either:

•      Have a new component, in a new Repo, with the Grid as a dependency
  

•      Keep it as is and people can the use the `<Grid/>` or the `<ExcelGrid/>`
  

•      Add module paths under lib, so you’d have lib\Grid and lib\ExcelGrid
  

If you can see a way to do this in more of a behaviour based way (using mixins / controllers I’d assume), that’d be great

For editors, with a few tweaks to allow passing full objects to the cellRenderer, that felt do-able without touching the Grids internals, cell selection though didnt

[bakesteve]

@jackofseattle no worries. and size is always important!
Not sure what your build pipeline is - but if you are using browserify/webpack/etc then if you just require('./Grid') you wont get any more than a few bytes extra with the extra keyboard navigation stuff

[bakesteve]

@andreypopp
Any initial thoughts on this?
If it helps can split out the gulp vs tests vs navigation vs v11 stuff?

On the main part of adding selection onto thegrid by click or keyboard, I've also done a attempt at a version as it feels more of a behaviour than anything else but had 2 key issues:

• mixins can't really add events without falling outside of the react event system.this means we lose all the normalization and GC optimization, and its critical for this as we are basically catching clicks and keys. Componentisation doesn't have those drawbacks. But somehow seems less elegant.
• assuming you want to mark cells to indicate the selected state, we need to pass that through the component tree. For use cases without selection that feels odd (though not going to have any measurable performance or size issues). I tried using cell renderers to get round this, but they don't have enough info to test if selected (no context of their row), and also need a way to access the array of selected cells. We could do that using stores (ie flux) or a mixin that accesses this via a closure. Neither felt very neat.
  My other though on that was just to stuff it onto the data we render. Which leads to another thought:

At the moment, a cell renderer gets the column and the value of your data object/array
For complex renderers we may need more info. Should we just let people stuff an object here? So data[row][colKey] ={
Value:123,
CurrencyCode:"usd",
Styles: ...
Cx: ...
}
That feels the most flexible to me and keeps the basic use case of passing in a flat object simple. Only extension I can think of is to allow value to be a function,to allow better composition ad functional lenses etc?
In the case of passing in if a cell is selected, it would mean we could have:
function() {
return {
value:"abc",
IsSelected:function(column, rowData) {
Return Selcted && Selected[rowData.key] && Selected[rowData.key][column.key]
}
}
}

The (potential) issue I can think of here is working out should component update at the cell level, as without evaluating both the lens and isSelected() you can't tell.

Let me know your thoughts on directions for this, as well as if it would help to break out this pr
One thing for us that will help is using grunt/gulp not makefiles

Cheers
steve

[andreypopp]

Why removing ScrollShim? On Mac OS X scrolling becomes flaky for some reason, ScrollShim helped with that. Though maybe there's a better solution.

[bakesteve]

Given the grid has now changed since we have created our master, going to close this down and submit properly (ie feature based pull requests, not 1 big behemoth)