Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] Support Multiple Grid in 1 View - Major Release with Breaking Changes #31

Closed
29 tasks done
ghiscoding opened this issue May 22, 2018 · 5 comments
Closed
29 tasks done

Comments

@ghiscoding
Copy link
Owner

ghiscoding commented May 22, 2018

After working on supporting Multiple Grid in same View, I found out that all the work from the start was to use Services as Singleton (services which are exposed by Angular-Slickgrid through Depency Injection (DI)). The first code change that was made to support multiple grid was to change the Singleton Services to non-Singleton, however that breaks all the Services exposed by Angular-Slickgrid.

So we need a new Major Release, below are the things that I have always put aside and that will be addressed before releasing a new Major Version (aka version 1.x)

Task List

  • 1. GridEvent Service, no longer references the 2nd argument gridOptions
  • 2. a lot of the Services instantiated in aurelia-slickgrid ViewModel use an init() which usually ask for init(grid, gridOptions, dataView) but once we get the grid object, we actually have access to the gridOptions, so we can simplify this a lot (closed by PR #28).
  • 3. remove all deprecated code (just search for the word deprecated), code change could be renamed or re-implemented (commit)
    • a. onBackendEventApi was re-implemented and replaced by backendServiceApi
  • 4. change all Singleton Services to non-Singleton
  • 5. Filter could possibly use only searchTerms array instead of doing multiple logic to deal with both searchTerm (singular) and the searchTerms array (commit and commit).
  • 6. Remove all previous reference of selectOptions in all Filters (commit).
  • 7. Editors/Filters should use a common Column property and possibly remove the filter Column property (commit).
  • 8. remove FormElementType from Filters, it should only use FilterType (commit)
    • a. the FormElementType is actually not used anywhere else I think, so might be better to delete it completely
  • 9. BackendService API onFilterChanged function and the Filter Service onFilterChanged Event Emitter, should not be named the same, it is rather confusing when troubleshooting, maybe change function to processOnFilterChanged (commit)
  • 10. remove exportWithFormatter from GridOptions.interface since it is now part of exportOptions (commit)
  • 11. GridMenu has both hideX and showX flags, they should all be hideX since that is the default name in the external plugins (commit)
  • 12. Possibly rename the GridExtraService to simply GridService or DataGridService
  • 13. Not exporting all Services in the Angular-Slickgrid module can break user who created their own BackendServiceApi, for example swt-grid-component
    • a. remove all Services from the module export since they aren't singleton anymore (commit)
  • 14. A lot of the Editors properties (like collection) are available only from the generic params property which removes the benefit of intellisense and type check. This goes with number 7. (commit)
  • 15. Expose all SlickGrid Events the same way they are currently exported in Aurelia-Slickgrid (commit and commit)
    • a. update Event Wiki, for now more info can be found in the Aurelia Wiki
      • SlickGrid Event onClick can be used in the View as (sg-on-click)="handleClick($event.detail.eventData, $event.detail.args)"
      • DataView Event onRowCountChanged can be used as (sg-on-row-count-changed)="handleRowCountChanged($event.detail.eventData, $event.detail.args)"
    • b. update the Example 3 (Editors) to demo this new way of using sg-on-click=""
  • 16. Remove the initOptions from the BackendService, that was replaced by init (commit)
  • 17. GridExtraUtil contain only 1 function, it would be better to move it into the renamed GridService and delete GridExtraUtil completely (commit and commit)
  • 18. Should pass i18n as a Grid Options instead of a generic params grid option and fix a column picker issue (commit)
  • 19. add Percent (divide by 100, they display %) and PercentSymbol (only add "%" suffix symbol) (commit)
  • 20. Add a sample for multiple grids in 1 view (commit)
  • 20. Update all the Wikis and there's a lot
  • 21. Update all the GitHub demo examples

Migration Guide will be available here

@ghiscoding
Copy link
Owner Author

@sabeurch
I need your input on number 13.
If you read the title of this issue, you will end up seeing that I am in the process of doing a Major version with breaking changes. The breaking piece is that users won't use Dependency Injection anymore to use the Angular-Slickgrid Services, because they are no longer Singleton.

All that to say, if I look at my Services using the BackendServiceApi (which are GraphqlService and GridOdataService). They are not using using any DI of Angular-Slickgrid, but your demo (Example 13) as all the Services in the constructor with DI. Is it just because you did a copy + paste? or do you use any of these Services? I don't want to break your custom BackendServiceApi but I need an answer rather soon on this.

@sabeurch
Copy link
Contributor

sabeurch commented May 23, 2018

@ghiscoding
I confirm that we can remove all DI in the contructor of SwtCommonGridComponent except the HttpClient one which is supposed to be used in the logging framework (even if not implemented, but looks wise to keep it, ie as if we are logging into the server the errors).
BTW, for a time ago I wanted to take a talk with you about the expanding number of dependency injections in the AngularSlickgridComponent's constructor, in fact you can group all services into a unique service and inject it solely. But now, since you are going to "provide" services from within the component itself, then forget my suggestion.
Are you planning to move all singleton services into non singletons ? I think yes, since variables such as :
private _dataView: any; and private _grid: any; are shared .

@ghiscoding
Copy link
Owner Author

ghiscoding commented May 23, 2018

@sabeurch
I only wanted to confirm the Angular-Slickgrid DI in your custom implementation, HttpClient is not part of Angular-Slickgrid, so it's all good. I won't be exposing the Services anymore from the outside, instead they will be available though 1 Angular-Slickgrid instance (an object containing references to all the Services). This won't remove the number of services, but they won't be available as DI anymore, simply through the instance, this way each grid (if multiple grids in same view) will have it's own services and one won't affect the other.

Are you planning to move all singleton services into non singletons ? I think yes, since variables such as: private _dataView: any; and private _grid: any; are shared .

The reason of this Major Version is because I want to support multiple grids and that requires moving all Services from singleton to non singleton (hence the Major version with breaking changes)

NOTE

  • Also read number 16. this one will concern you with your custom Backend Service API. Not a huge thing though, just rename initOptions to init. The function signature changed, so I had to use another function name.
  • number 9. will also concern you and you can look at the commit, I also updated your custom Swt Components since it's now part of my repo ;) you can see the commit

@mohit4834
Copy link

Hello @ghiscoding,
Greetings!
Does our angular slickgrid supports delete of multiple rows after i select multiple rows using our row selection. As i am not able to do it.
Major Issue i am facing is once i provide index of the rows i selected from the grid. After 1st delete index got changed and there after it deletes the row which are not selected.

Thanks,
Mohit Goyal

@ghiscoding
Copy link
Owner Author

@mohit4834
Your issue is different than the original issue, you should open a new issue if need be.
There's a delete example in the Editor Example and the code is shown here

Repository owner locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants