update dropdown formatter example #309

jeremielodi · 2019-10-10T12:33:02Z

using a separate service to manage dropdown.

ghiscoding · 2019-10-10T13:20:46Z

src/app/examples/grid-angular.component.ts

@@ -11,13 +11,14 @@ import {
  Formatters,
  GridOption,
  OnEventArgs,
-} from './../modules/angular-slickgrid';
+} from 'angular-slickgrid';


you cannot do that here, you need to put back previous code. This is the lib and we need to reference it, it is not installed as a node_modules but instead is used directly

you're right

ghiscoding · 2019-10-10T13:21:58Z

src/app/examples/grid-angular.component.ts

+        args,
+        actionColumnIndex: 6, 
+        parent : this,
+        $


there's no need to pass the $, you can use declare var $: any; in the Service.

codecov-io · 2019-10-10T13:35:53Z

Codecov Report

Merging #309 into master will decrease coverage by 0.44%.
The diff coverage is 32.7%.

@@            Coverage Diff             @@
##           master     #309      +/-   ##
==========================================
- Coverage   97.28%   96.85%   -0.43%     
==========================================
  Files         135      137       +2     
  Lines        7644     7693      +49     
  Branches     2593     2598       +5     
==========================================
+ Hits         7436     7450      +14     
- Misses        208      243      +35

Impacted Files	Coverage Δ
src/app/modules/angular-slickgrid/index.ts	`100% <100%> (ø)`	⬆️
...ular-slickgrid/modules/angular-slickgrid.module.ts	`100% <100%> (ø)`	⬆️
.../app/modules/angular-slickgrid/formatters/index.ts	`100% <100%> (ø)`	⬆️
...ngular-slickgrid/formatters/bsDropdownFormatter.ts	`100% <100%> (ø)`
...rc/app/modules/angular-slickgrid/services/index.ts	`100% <100%> (ø)`	⬆️
...s/angular-slickgrid/services/bsDropdown.service.ts	`14.64% <14.64%> (ø)`
...dules/angular-slickgrid/sorters/sorterUtilities.ts	`100% <0%> (ø)`	⬆️
...ckgrid/filter-conditions/executeMappedCondition.ts	`100% <0%> (ø)`	⬆️
...pp/modules/angular-slickgrid/services/utilities.ts	`100% <0%> (ø)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e90077...de1758d. Read the comment docs.

ghiscoding · 2019-10-10T15:24:18Z

src/app/examples/grid-angular.component.ts

@@ -18,6 +18,8 @@ import { CustomAngularComponentEditor } from './custom-angularComponentEditor';
 import { CustomAngularComponentFilter } from './custom-angularComponentFilter';
 import { CustomTitleFormatterComponent } from './custom-titleFormatter.component';
 import { FilterNgSelectComponent } from './filter-ng-select.component';
+import { DropDownService } from './dropdown.service';


Your new Service doesn't seem to be part of the providers of the App Module, is that something you forgot to do? You don't seem to instantiate that Service but instead using it as a static Service, I'm not sure that is a good idea since you won't be able to use Dependency Injection in the future

jeremielodi · 2019-10-11T05:34:19Z

The last thing we can do is to create dropdownFormatter like you did iconFormatter defined at src\app\modules\angular-slickgrid\formatters\iconFormatter.ts

ghiscoding · 2019-10-11T06:45:53Z

If you add something to the library code itself (the Formatter), you will need to add Unit Tests as well. There isn't any unit tests for the examples but all the lib is fully tested (what I have for the Examples are some E2E tests). You can take a look at the Formatter __tests__ folder. I put a lot of effort in adding all necessary tests, so I hope you would do the same so that the test coverage doesn't get lower.

I see that your last commit decreased the test code coverage

- Coverage   97.28%   97.22%   -0.06%

I'm also wondering if the name you chose for the Formatter is a little too generic, it doesn't represent that it's a Bootstrap Dropdown component. I'm not sure of a better name, but I'd like to make it a bit clearer that it's not a regular dropdown... maybe bsDropdownFormatter or bootstrapDropdownFormatter, I'll let you choose the final name

jeremielodi · 2019-10-11T10:24:51Z

I did not realize that you use boostrap3, all my updates were working fine in my project but when I tried to create a PR here there was bugs..

ghiscoding · 2019-10-11T16:04:10Z

My library works with both version of Bootstrsap 3-4. The lib itself was written with Bootstrap 3 but when I want to test BS4, I use the Angular-Slickgrid-Demos for BS4. Usually just passing the css classes for both versions makes it work for both version.

jeremielodi · 2019-10-11T17:39:58Z

Did you test my updates, does it work fine?

ghiscoding · 2019-10-11T19:44:01Z

Didn't have time yet, this is mostly just an example

ghiscoding · 2019-10-12T05:21:30Z

src/app/modules/angular-slickgrid/services/bsDropdown.service.ts

+
+
+interface DropDownServiceParams {
+  grid: AngularGridInstance,


You don't need to pass this grid object, you can get it instead from the SharedService, see this line as example

constructor(... sharedService: SharedService) {} ... this.sharedService.grid;

ghiscoding · 2019-10-12T05:24:01Z

I see you changed the code a lot and added a new Service, that is all fine but again I'll ask to add necessary Jest Unit Tests for this new Service. I didn't have a chance to try this yet, I'll give it a try next week. Thanks

ghiscoding · 2019-10-15T13:44:48Z

src/app/modules/angular-slickgrid/services/__tests__/bsDropdown.service.spec.ts

+  describe('Render method', () => {
+    const service = BsDropDownService;
+    it('should show check is the dropdown is displayed after clic', () => {
+      expect(true).toBe(true);


I hope you will eventually add more tests than this, because I won't accept this PR as it is. Please provide sufficient Jest Unit Tests before I can review and accept this PR

ghiscoding · 2019-10-17T14:20:44Z

Superseded by #314

@jeremielodi
I took your PR and fixed a few things, added Jest Unit Tests and Cypress E2E test for your new Action Dropdown. I'll release a new version today.

There is 1 major thing to note about your code, you cannot export the SharedService from the Angular-Slickgrid Module, the reason is because that service (and others) have to be Non-Singleton. If you make them Singleton and you have 2 or more Grids in your window and you export the service as Singleton, then using a service in 1 grid could/will impact the next grid... So that is the reason the SharedService and a few other Services are only available from the (onAngularGridCreated)="angularGridReady($event)"

jeremielodi · 2019-10-17T14:34:18Z

Thanks

jeremielodi · 2019-10-17T15:10:34Z

@ghiscoding, I wonder if it's possible to have the component(page) context inside a formatter.
That way it will be easy to render templates in grid's cell.

ghiscoding · 2019-10-17T15:40:07Z

You can refresh your code and play with it and make other contributions if you wish in the future. I'm about to release a new version (in the next few minutes) and I will go with the code I have now. I'm certainly open for PR that might enhance the lib and/or the examples. Thanks

ghiscoding · 2019-10-17T15:51:00Z

Also up vote ⭐️ if you like the lib and haven't up voted yet, thanks 😸

ghiscoding · 2019-10-17T18:25:49Z

Found out that the dropdown was not working with Bootstrap 4, I made a commit to fix that in the BsDropdownService but I'll wait couple days before releasing a new version, just in case something else comes up.

EDIT

I actually refactored the Service again and added almost full test coverage now with this commit

ghiscoding · 2019-12-07T00:04:14Z

@jeremielodi
You might be interested in 2 new Context Menu plugins that I'm currently working on here. I'm currently developing them as SlickGrid (core) Plugins, once that is done I will integrate them into Angular-Slickgrid (might fall after Christmas).

jeremielodi mentioned this pull request Oct 10, 2019

Any example of a dropdown menu in a particular cell #304

Closed

ghiscoding reviewed Oct 10, 2019

View reviewed changes

jeremielodi force-pushed the dropdown-update branch from 536f0a0 to b477ecd Compare October 10, 2019 13:35

jeremielodi force-pushed the dropdown-update branch 2 times, most recently from 4b37218 to 682e9da Compare October 10, 2019 13:44

ghiscoding reviewed Oct 10, 2019

View reviewed changes

jeremielodi force-pushed the dropdown-update branch 2 times, most recently from df23679 to 8c5fb14 Compare October 11, 2019 05:26

jeremielodi force-pushed the dropdown-update branch 2 times, most recently from 6bc77b3 to 982cacd Compare October 11, 2019 06:12

jeremielodi force-pushed the dropdown-update branch from 982cacd to 5973431 Compare October 11, 2019 10:20

jeremielodi force-pushed the dropdown-update branch from 5973431 to afd1c84 Compare October 11, 2019 10:47

jeremielodi force-pushed the dropdown-update branch from afd1c84 to 5f639fd Compare October 11, 2019 18:12

update bootstrap dropdown formatter

ef7802a

jeremielodi force-pushed the dropdown-update branch from 5f639fd to ef7802a Compare October 11, 2019 18:17

ghiscoding reviewed Oct 12, 2019

View reviewed changes

update slickgrid module( add bsdropdown service and formatter)

a038716

jeremielodi force-pushed the dropdown-update branch from 37d1561 to a038716 Compare October 13, 2019 06:49

jest test

de1758d

ghiscoding reviewed Oct 15, 2019

View reviewed changes

ghiscoding mentioned this pull request Oct 17, 2019

feat(bootstrap): update Action Bootstrap Dropdown formatter example #314

Merged

ghiscoding closed this Oct 17, 2019

ghiscoding mentioned this pull request Dec 6, 2019

feat(menu): add Cell Menu & Context Menu features #353

Merged

32 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update dropdown formatter example #309

update dropdown formatter example #309

jeremielodi commented Oct 10, 2019

ghiscoding Oct 10, 2019

jeremielodi Oct 10, 2019

ghiscoding Oct 10, 2019

jeremielodi Oct 10, 2019 •

edited

Loading

codecov-io commented Oct 10, 2019 •

edited by codecov bot

Loading

ghiscoding Oct 10, 2019

jeremielodi commented Oct 11, 2019

ghiscoding commented Oct 11, 2019 •

edited

Loading

jeremielodi commented Oct 11, 2019

ghiscoding commented Oct 11, 2019

jeremielodi commented Oct 11, 2019

ghiscoding commented Oct 11, 2019

ghiscoding Oct 12, 2019

ghiscoding commented Oct 12, 2019

ghiscoding Oct 15, 2019

ghiscoding commented Oct 17, 2019

jeremielodi commented Oct 17, 2019

jeremielodi commented Oct 17, 2019 •

edited

Loading

ghiscoding commented Oct 17, 2019

ghiscoding commented Oct 17, 2019

ghiscoding commented Oct 17, 2019 •

edited

Loading

ghiscoding commented Dec 7, 2019

update dropdown formatter example #309

update dropdown formatter example #309

Conversation

jeremielodi commented Oct 10, 2019

ghiscoding Oct 10, 2019

Choose a reason for hiding this comment

jeremielodi Oct 10, 2019

Choose a reason for hiding this comment

ghiscoding Oct 10, 2019

Choose a reason for hiding this comment

jeremielodi Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

codecov-io commented Oct 10, 2019 • edited by codecov bot Loading

Codecov Report

ghiscoding Oct 10, 2019

Choose a reason for hiding this comment

jeremielodi commented Oct 11, 2019

ghiscoding commented Oct 11, 2019 • edited Loading

jeremielodi commented Oct 11, 2019

ghiscoding commented Oct 11, 2019

jeremielodi commented Oct 11, 2019

ghiscoding commented Oct 11, 2019

ghiscoding Oct 12, 2019

Choose a reason for hiding this comment

ghiscoding commented Oct 12, 2019

ghiscoding Oct 15, 2019

Choose a reason for hiding this comment

ghiscoding commented Oct 17, 2019

jeremielodi commented Oct 17, 2019

jeremielodi commented Oct 17, 2019 • edited Loading

ghiscoding commented Oct 17, 2019

ghiscoding commented Oct 17, 2019

ghiscoding commented Oct 17, 2019 • edited Loading

ghiscoding commented Dec 7, 2019

jeremielodi Oct 10, 2019 •

edited

Loading

codecov-io commented Oct 10, 2019 •

edited by codecov bot

Loading

ghiscoding commented Oct 11, 2019 •

edited

Loading

jeremielodi commented Oct 17, 2019 •

edited

Loading

ghiscoding commented Oct 17, 2019 •

edited

Loading