Feature datagrid selections

[sccolbert]

@blink1073 @afshin @SylvainCorlay @jasongrout @ellisonbg

This is ready to discuss from an API perspective. This adds a formal selection model and related APIs to the grid. Checkout the branch and play with the datagrid example. Full mouse and keyboard selection is supported. I've also jazzed up the rendering a bit via scroll shadows.

There's still a couple edge-cases/known-issues that I'm working on, so it's not quite ready to be merged.

[ellisonbg]

Thanks! Is there special build magic required? I did a npm install; npm run build and got errors (can include the error message here if this should work).

[blink1073]

This is great! I found myself wishing for @ian-r-rose's PR bot for Phosphor examples 😀. Today got away from me, but I'll kick the tires on this tomorrow.

[afshin]

@ellisonbg, try using yarn instead of npm

npm install --global yarn # If you don't have yarn already
cd phosphor
yarn
yarn install
yarn build
cd examples/example-datagrid
python -m http.server

[sccolbert]

From a clean checkout, all you should need is:

lerna bootstrap lerna run build

[nrbgt]

Oooh, would :heart: this! Need to take a look at the implementation!

I built something like this on wxyz (a pile of crazy widgets) () but would love to shed some complexity.

[mbektas]

@sccolbert this is looking great! one little comment: on MacOS multiple selections are usually done using Cmd + click instead of Ctrl + click. It would be great if you can support that. This is how I was detecting in my implementation https://github.com/mbektasbbg/phosphor/blob/selections_dev/packages/datagrid/src/datagrid.ts#L4095-L4098

[ellisonbg]

Thanks, got it working with lerna/yarn. The use experience is super solid - very nice!

On Fri, Aug 30, 2019 at 7:08 AM Mehmet Bektas notifications@github.com wrote:

@sccolbert https://github.com/sccolbert this is looking great! one little comment: on MacOS multiple selections are usually done using Cmd + click instead of Ctrl + click. It would be great if you can support that. This is how I was detecting in my implementation https://github.com/mbektasbbg/phosphor/blob/selections_dev/packages/datagrid/src/datagrid.ts#L4095-L4098

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/phosphorjs/phosphor/pull/421?email_source=notifications&email_token=AAAGXUFQPEA5XHCIX2TUEW3QHESWJA5CNFSM4ISGPHD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5RYI5A#issuecomment-526615668, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAGXUFAY4B4OJH4SJVJ4F3QHESWJANCNFSM4ISGPHDQ .

-- Brian E. Granger

Principal Technical Program Manager, AWS AI Platform (brgrange@amazon.com) On Leave - Professor of Physics and Data Science, Cal Poly @ellisonbg on GitHub

[sccolbert]

Brian, thanks! Ctrl/Cmd was an oversight on my part. Will fix!

[afshin]

This is awesome! I've been playing with it and showing people today.

I noticed a glitch when the selection goes over the edge of the fixed columns in the trillion row table.

[sccolbert]

@afshin Yeah, that's one of the "known issues" I was talking about 👍

[sccolbert]

Also, please take a look at the user-facing APIs with how the selection models and key/mouse handlers are setup on the grid. The grid does not provide default implementations of those things (because it would create a circular type dependency), so the user must install them after they instantiate a grid.

Finally, if you can think of a legitimate use case that cannot be implemented with the given abstract APIs, please let me know before we get locked into this.

Thanks!

[sccolbert]

@mbektasbbg The cmd issue on Mac should be fix. I can't test it myself though. Can you please give it a go?

[mbektas]

thanks @sccolbert , i just tested and it is working fine!

[sccolbert]

@afshin That edge case is now handled. I've also implemented auto-scrolling when you drag the mouse outside the bounds of the grid. I think this PR is now complete. Let me know if you want any API changes, otherwise I will merge this very soon.

[afshin]

I think this is a really solid API. I don't have any changes to propose, but I did have some questions that I asked in-line.

One thing that is outside the scope of this change but I wondered about is in the data model:

How do you feel about changing these signatures make region be optional and putting it at the end of the parameter list, defaulting it to 'body'. The idea would be that the simplest case is a model that returns body data.

I don't feel strongly about this, either, but it did strike me as curious that region was the first parameter.

data(row: number, column: number, region?: DataModel.CellRegion = 'body'): any;

metadata(column: number, region?: DataModel.CellRegion): DataModel.Metadata

[blink1073]

Here's an interesting one: hold down Ctrl (or Cmd I assume), and repeatedly select a row or column header. The column is progressively shaded darker and darker:

[nmichaud]

@sccolbert This looks good. API is powerful enough to support my usecase for ticking sorted data.

[sccolbert]

How do you feel about changing these signatures make region be optional...

I'd be :-1: on that. The implementor of a DataModel must understand that there are different regions, because the DataGrid will request data for all regions.

[sccolbert]

Here's an interesting one: hold down Ctrl (or Cmd I assume), and repeatedly select a row or column header. The column is progressively shaded darker and darker:

This is expected behavior. Holding ctrl will initiate multiple selections. So you are simply creating multiple selections atop one another. Excel will deselect the column in this case. It will also intersect (instead of union) a selection that starts over another selection. I chose not to implement this behavior as I question its real utility. We can add it prior to 1.0 if it proves to be needed.

[blink1073]

🚀 💥 🔥 ✨