refactor(services): remove gridOptions and columnDefinitions

[ghiscoding]

• removed all references from all init/attach Services

Note it does not include the refactoring of gridEventService.attachOnCellChange and gridEventService.attachOnClick since that was already done in a separate PR #51

@jmzagorski
It's a long time that I wanted to cleanup these arguments and only use grid and dataView objects. We can get options and columnDefinitions from the grid and so we should do that everywhere.

I tested all the grids with all features and everything seems to work as usual.
Hopefully this will help in issue #36 (multiple grids in one view)

Note, this PR has to be merged only after PR #51 , hopefully without conflicts afterward but it's possible and will address at that time if that happens.

[jmzagorski]

i think it is good to reduce the number of parameters since most of them can be retrieved from slickgrid. This may help with multiple grids if the consumer of the service remembers to call init every time a method on that service is called to ensure the service has the correct grid (which could be cumbersome at first, but I think that is fine for a first iteration)

My last thought on this is storing the the grid options in _gridOptions on init. It could be possible that something changes the grid options from the time init is called and the time the next method is called, which would make the _gridOptions out of date. The only way to ensure you have the most current grid options is to call getOptions at the point you need the options.

[ghiscoding]

In that case, would you mind pulling the branch and adding the _gridOptions in the init as what you think is ideal or provide the line and code to change. Thanks

[jmzagorski]

I commit the changes to the services, see if it works for you. All i did was move the _gridOptions to a readonly property that always gets the grid options from slickgrid instead of a local field.

The test i ran (not shown in the code) was to set a timeout in example11 that changes the enableColumnPicker to false. Then I clicked Add New Mocked Item with a debugger breakpoint in highlightRow to inspect the _gridOptions and see it was false which means the options are up to date. Sort of a silly test since I know the readonly _gridOptions always calls the this._grid.getOptions, but I always try to test.

[ghiscoding]

Oh wow ok, it's more than 1 line of code, I'm ok with the changes but I would like to test all examples before merging it. I'll do that in the coming days.

[ghiscoding]

I left some comments because I'm at work and I'm trying to apply the changes to my Angular-Slickgrid as well. I also saw that the new getter

private get _gridOptions(): GridOption {
    return (this._grid && this._grid.getOptions) ? this._grid.getOptions() : {};
  }

is not applied to all Services but it probably should be. So far I see these Services missing it

• GridStateService
• GridOdataService
• GraphqlService
• ResizerService

@jmzagorski
The Getter for the _gridOptions is nice, I wonder why aren't we doing the same for the columnDefinitions in all Services?

EDIT
I went ahead and made all changes, we now have Getters for _gridOptions and also for _columnDefinitions. This will be in good shape for this new issue that I've open #57

@jmzagorski
Any other comments before I merge this big PR?

[jmzagorski]

sorry just saw this. Looks good!