version 2.x refactoring to support Multiple Grid

[ghiscoding]

Here's a list of what cannot be done currently in version 1.x until a new major release comes out 2.x which mean breaking changes

• 1. GridEvent Service, no longer references the 2nd argument gridOptions (closed by PR fix(eventService): use grid.getOptions for gridDefinition #51)
• 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 refactor(services): remove gridOptions and columnDefinitions #55).
• 3. remove all deprecated code (just search for the word deprecated), code change could be renamed or re-implemented
  • a. onBackendEventApi was re-implemented and replaced by backendServiceApi (commit)
• 4. remove possibly unnecessary global Event Aggregators (EA) by Element Dispatch (ED)
  • a. to be discussed, having EA globally might help in some cases, in my current project I have a left side menu that when triggered needs to resize the grid (autoResize doesn't work in that case) and I don't think it would be possible without an EA.
  • b. use asg for Aurelia Events and sg for SlickGrid/DataView Events & rename eventPrefix to slickgridEventPrefix (commit)
• 5. Filter could only use searchTerms array instead of doing multiple logic to deal with searchTerm (singular) and searchTerms array (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.
• 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 Event and function should not be named the same, it is rather confusing when troubleshooting, maybe change function to processFilterChanged (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 (commit)
• 13. 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.
• 14. Remove the initOptions from the BackendService, that was replaced by init some time ago (commit)
• 15. GridExtraUtil contain only 1 function, it would be better to move it into the renamed GridService and delete GridExtraUtil completely (commit)
• 16. Should pass i18n as a Grid Options instead of a generic params grid option and fix a column picker issue (commit)
• 17. change all Singleton Services to non-Singleton (commit)
• 18. renamed onGridStateServiceChanged to onGridStateChanged
• 19. add multiple grids in a view (commit and commit)
• 20. change column.interface signature of onCellClick and onCellChange to expose event as first argument to keep consistent with SlickGrid, see issue [Bug]: adding onCellClick on editor column removes autoEdit , doesn't remove selectEditor #60
• 21. update Migration Guide
• 22. update all client-cli and GitHub demo samples
• 23. update Wikis and there's a lot

This list will be taken into consideration in the version 2.x which will be a Major and breaking version.
Simply keeping this for reference as of now.

Also, don't expect a version 2.x any time soon.
This will be addressed only when we decided that all features that we want are in place and we want to get rid of all properties and naming that weren't of good use in 1.x.

Migration Guide will be available here

[ghiscoding]

Migration Changes Guide from version 1.x to 2.x
The list is starting to be too big, so I created a Wiki for it

[ghiscoding]

@jmzagorski
How do you want to address number 7. and 13.

[jmzagorski]

For number 7 i think you wanted to keep the editor and filter configuration separated in the columnDef. Probably a good idea since both accept a collectionFilterBy property and I have a grid that dynamically filters the params.collection by changing the columnDef.params.collectionFilterBy on cell click, while leaving the filter.collection alone. Not sure if that can be accomplished if both the editor and filter use the same configuration?

That would mean leave the editor on the params or move them to an editor specific property (in reference to number 13)

[ghiscoding]

@jmzagorski
I would like to put some Types for Editors, not just for Filters. The problematic is even though we can use filter, we can't use editor since it is used internally by SlickGrid itself. We could call it editorOptions though, and if so, we could potentially rename filter to filterOptions to make them equal (however, I understand that would create more refactoring in the migration, but I'm ok with that).

The end goal is to get some Types (and intellisense when the coding editor supports it, like VScode). I wanted to wait until Filters and Editors were stable enough, I think they are now, and for version 2.x before revisiting them. I think we have everything we need to make a decision, which I kinda need rather soon because I already started the Major version work in my Angular repo.

I don't think that leaving all of the Editors stuff in params is a good idea, especially if we want more TS Typing.

[jmzagorski]

Here are a few other options I could think of to brainstorm about:

1. Instead of filterOptions, editiorOptions etc, make a property called options and then put filter and editor as properties (columnDef.options.filter, columndDef.options.editor)

2. Make your public API columnDef.editor a Editor | EditorOptions so it is similar to the filter. Then in AureliaSlickGridCustomElement move the editor option properties to another property on the columnDef and put the actual Editor interface on the original columnDef.editor for slickgrid to use.

[ghiscoding]

I was thinking about your number 1. too, the downside is that it makes the property calls longer. Unless you want to merge Editor/Filter properties and only separate the ones that you want? But that might be confusing for collection stuff

This kinda mean that Editor and Filter will have the exact same collection behavior. Is that always wanted?

options: {
       collection: [ 
         { value: '', label: '' }, 
         { value: true, label: 'true' }, 
         { value: false, label: 'false' },
         { value: undefined, label: 'undefined' }
       ],
       collectionFilterBy: {
          property: 'effortDriven',
          operator: OperatorType.notEqual,
          value: undefined
       },
       collectionSortBy: {
          property: 'effortDriven',    // will sort by translated value since "enableTranslateLabel" is true
          sortDesc: false,             // defaults to "false" when not provided
          fieldType: FieldType.boolean // defaults to FieldType.string when not provided
       }, 
       type: FilterType.multipleSelect
   }

Your number 2. I'm not sure to follow you on how to fake editor. Are you saying to replace editor before passing the column definitions to the new Slick.Grid? I feel that it's kind of a patch

[jmzagorski]

I think combing it might be troublesome, but that is just a guess. For example, in my app I dynamically filter the singleSelectEditor when a cell is clicked, but the header filter still shows all the values from the same collection. I do this because each row can only have a subset of values from the entire collection based on another value in the same row. For example:

    onCellClick: ({ columnDef, dataContext }) => {
      columnDef.params.collectionFilterBy = {
        property: 'jobTitleId',
        value: dataContext.ownerJobTitleIds,
        operator: OperatorType.contains
      };
    },

So if this was changed every time a cell was clicked, will this affect the header filter? I know the header filter is rendered early on so it might not affect it unless the header filter updates while the user is using the grid.

As for number 2, yes it is sort of "hacky" i guess. The only thing it does is makes your public API cleaner and not tied to slickgrid anymore. Here is a brief example of how it could work without changing the underlying column definitions:

    this.grid = new Slick.Grid(`#${this.gridId}`,
       this.dataview,
       // if we made the Editor class property name "slickEditor"
       this._columnDefinitions.map(c => ({ ...c, editor: c.editor.slickEditor })),
       this.gridOptions);

[ghiscoding]

hmm maybe with the spread operator it doesn't look too bad. What is c.editor.slickEditor? I mean where does that come from? Aren't we just supposed to pull whatever the user added in editor and empty it out afterward we're done keeping it in a temp area that will be used by our EditorService?

[jmzagorski]

c.editor.slickEditor would be what columnDef.editor is today, i just gave it a random name so probabaly should use another name if we decided to use this method (maybe call it type like filter.type does?). For example:

Today

{
    editor: Editors.date,
    field: 'due',
    filter: {
      type: FilterType.compoundDate
    },
    filterable: true,
    formatter: Formatters.dateIso,
    id: 'due',
    name: 'Due',
    sortable: true,
    type: FieldType.dateIso
  }

Brainstorm

{
    editor: {
      slickEditor: Editors.date,
     // ... other editor options here
    },
    field: 'due',
    filter: {
      type: FilterType.compoundDate
    },
    filterable: true,
    formatter: Formatters.dateIso,
    id: 'due',
    name: 'Due',
    sortable: true,
    type: FieldType.dateIso
  }

[ghiscoding]

but where is the part to swap the property in a temp variable? Isn't your .map supposed to be reassigned to column definitions after you're done? SlickGrid uses editor for it's Editor Factory, so we need to empty it out before creating the grid itself. If we use editor that can be renamed to a temp variable (let say editorOptions), isn't it suppose to look like the following?

this.grid = new Slick.Grid(`#${this.gridId}`,
       this.dataview,
       // if we made the Editor class property name "slickEditor"
       this._columnDefinitions = this._columnDefinitions.map(c => ({ ...c, editor: { }, editorOptions: c.editor })),
       this.gridOptions);

BTW, I'd like to have the brainstorm done by the end of this week and implement it in the weekend and then in my Angular repo on Monday. I need multiple grids in my project, like now lol

[jmzagorski]

oh yeah i forgot that part, and I forgot about the editorFactory property on gridOptions. So we can blank out the editor and put our factory on gridOptions.editorFactory.getEditor or reassign the editor and editorOptions on the fly

If you were to use editorOptions the reassignment would look like this:

this.grid = new Slick.Grid(`#${this.gridId}`,
       this.dataview,
       // if we made the Editor class property name "slickEditor"
       this._columnDefinitions = this._columnDefinitions.map(c => ({ ...c, editor: c.editor.slickEditor, editorOptions: { ...c.editor } })),
       this.gridOptions);

If you were to use editor factory it would look like this:

-for (const c of this._columnDefinitions) {
-      if (c.editor) {
-        c.editor = Factory.of(c.editor).get(this.container);
-      }
-}
+ this.gridOptions.editorFactory {
+  getEditor: column => Factory.of(this.columnDefinitons.find(c => c.id === +column.id).editor.slickEditor).get(this.container)
+}

this.grid = new Slick.Grid(`#${this.gridId}`,
       this.dataview,
       // if we made the Editor class property name "slickEditor"
+       this._columnDefinitions = this._columnDefinitions.map(c => ({ ...c, editor: null, editorOptions: { ...c.editor } })),
       this.gridOptions);

[ghiscoding]

When I said Editor Factory, I really meant SlickGrid internal factory which is the column editor property, you can see it used here by the getEditor() function. But yeah we also have our own Editor Factory, which you did an excellent job at implementing :)

The editor: c.editor.slickEditor still confuses me, I don't know what it does since I believe the editor should be left empty or null as you pointed out in 2nd sample.
BTW, editorFactory is also a column property used internally by SlickGrid here, so we can't use that option either :P

Which one do you think would be the best option to go with?

[jmzagorski]

If the editor is left null then we must implement the editorFactory. If we do not want to use a factory then we must set the editor property for slickgrid to use as we do today. editor: c.editor.slickEditor is a made up property for this example if we were to have your API use c.editor for the actual editor (e.g DateEditor, TextEditor etc) and the editor options.

{
    // today we do 
-   editor: Editors.date
    // but if you wanted to use editor as an object instead for the actual editor class & options
+   editor: {
+      slickEditor: Editors.date,
+      collection: this.editorCollection
     // ... other editor options here
    },
-    params: {
-      collection: this.editorCollection
-    }
  }

Not sure if that helps any. I don't have a preference as it stands now.

I was thinking about your number 1. too, the downside is that it makes the property calls longer.

I wouldn't worry about that if you use the destruction operator. The destruction would make it columnDef.options.editor.collection to options.editor.collection

[ghiscoding]

Dohhhh I feel like Omer Simpson now... I didn't realized that we do use the editor today lol
I guess we use it that way because that is what SlickGrid likes to see. Wow I'm sorry, I was missing an important piece since the start of the subject, my bad. I'm glad we talked because I would have missed it completely, now I understand the editor: c.editor.slickEditor it totally make sense

in that case, from your previous post. I think I would go with option number 1

this.grid = new Slick.Grid(`#${this.gridId}`,
       this.dataview,
       // if we made the Editor class property name "slickEditor"
       this._columnDefinitions = this._columnDefinitions.map(c => ({ ...c, editor: c.editor.slickEditor, editorOptions: { ...c.editor } })),
       this.gridOptions);

[ghiscoding]

@jmzagorski
I want to make sure that you see number 4.b and 4.c from the list on top. I would like to make a better separation of the dispatch events of SlickGrid versus the Aurelia-Slickgrid ones. Basically, I want SlickGrid to start with prefix sg, while the Aurelia-Slickgrid events to start with asg. This would help the user and us if there's any troubleshooting to do, to understand where the event is processed. Make sense?

[jmzagorski]

Makes sense yes. What does your html view on your left-side menu look like? You might be able to catch the event in your view if there is a parent element to your grid without using the EA.

[ghiscoding]

Ah the example that was written is with my current App, however it's almost identical to our demo with left-side menu, the only difference is that the left-side menu is collapsible. I can re-code our demo to look similar and add the collapsible. The idea is that the left-side menu and the right-side are containers, they don't know what goes in, neither do they have a relationship of Parent/Child, they are independent.

See the print screen below, the "x" is clickable which will collapse and only show the icons and not the text. When that "x" is clicked, it doesn't fire an DOM resize events, I think it's due to the fact that I only use a CSS property change and not an actual element resize... all that to say, I had to go with a Global Event Emitter (which in the Aurelia is an Event Aggregator)

[ghiscoding]

@jmzagorski
I am almost done with the refactoring of editor, I decided to go with the following

this._columnDefinitions = this._columnDefinitions.map((c: Column | any) => ({ ...c, editor: this.findEditor((c.editor && c.editor.type), c), internalColumnEditor: { ...c.editor } })),

Some notes

1. To make the editor to behave exactly like filter, I created a new interface EditorType which can be used inside the editor.type (exactly like filter).
   • it is a bit different than our last discussion since it doesn't take an actual SlickGrid editor, it is rather an enum of EditorType. This way it is similar to Filter and also brings Custom Editor in the play
2. instead of using editorOptions to internally swap the options, I decided to use a more familiar name internalColumnEditor which tells the user to not use that property
3. the function findEditor will be a Factory call in our Aurelia-Slickgrid, I couldn't find how to create a factory in Angular and so I just did a quick and dirty collection find instead of a factory.
4. the editor is of type ColumnEditor (just like filter was of type ColumnFilter) and this brings all the usual properties that were previously used the generic params
5. with the new EditorType, I was able to easily add a custom and created a Custom Editor and it... sweet
   • however, I can't new it since it has to a valid SlickGrid class (not an instance) because if I do, I won't get access to the args required in the constructor (provided internally by SlickGrid)
6. very important there is only 1 issue that still stand, the CollectionFormatter is using the Collection from the generic params and now it's no longer available. I can take it from the editor but wait I can't... why? because I don't know if the Collection is inside the editor or the filter object!?!
   • it's also not just the collection but also customStructure and possibly more in the future,
   • so for now, I left it inside params for the Example 3 (Editor) to still work.
   • I need your input for this one

The commit for all of that in the Angular-Slickgrid repo can be seen here and I will release a beta version in that repo in the coming days

[jmzagorski]

I am pretty sure CollectionFormatter should be from the editor because that function formats values selected from the editor.

however, I can't new it since it has to a valid SlickGrid class (not an instance) because if I do, I won't get access to the args required in the constructor (provided internally by SlickGrid)

Not sure what you mean by this. Are you newing up the editor class yourself?

[ghiscoding]

@jmzagorski
How about renaming the formatter to CollectionEditorFormatter which would get it's collection from the editor complex object while maybe leaving untouched CollectionFormatter and still associated to params

The new is what we currently do with a Custom Filter as shown here

{
  id: 'description', name: 'Description', field: 'description', filterable: true, sortable: true, minWidth: 80,
  type: FieldType.string,
  filter: {
    type: FilterType.custom,
    customFilter: new CustomInputFilter() // create a new instance to make each Filter independent from each other
  }
},

while a Custom Editor looks like this, the only difference is that we can't new it up

{
  id: 'title2', name: 'Title Custom', field: 'title',
  sortable: true,  minWidth: 70
  type: FieldType.string,
  editor: {
    type: EditorType.custom,
    customEditor: CustomInputEditor
  },
}

and here's a regular Editor with the new complex editor object

{
  id: 'prerequisites', name: 'Prerequisites', field: 'prerequisites',
  sortable: true,  minWidth: 100
  type: FieldType.string,
  editor: {
    type: EditorType.multipleSelect,
    collection: Array.from(Array(12).keys()).map(k => ({ value: `Task ${k}`, label: `Task ${k}` })),
    collectionSortBy: {
      property: 'label',
      sortDesc: true
    },
    collectionFilterBy: {
      property: 'label',
      value: 'Task 2'
    }
  }
}

[ghiscoding]

I update the migration guide for the editor complex object, you can see the guide

[jmzagorski]

okay I see what you mean with the CollectionEditorFormatter. If the user does not have an editor, they still might need to use the CollectionFormatter.

What about the CollectionFormatter checking if there is an editor then using the editor.collection property and if not fall back to the params.collection?

[ghiscoding]

@jmzagorski
I was thinking about the fallback but then if the user also has the collection inside the filter, then what? which one has priority and such? Also the if condition is gonna be huge, take a look at the if code now. I would prefer to simply create multiple formatters, it's easy to create and doesn't add much more code, it's also more focused and less confusing

[ghiscoding]

@jmzagorski
That was an amazingly long day, it took me couple of hours to release all of this and it seems that NPM has some issues showing my Beta version. I had similar issues on my work computer, so I guess I will wait few hours to see. NPM says that it got publish for version 2.0.0 with a Beta tag but I still don't see it

Anyhow, here's the last info on the project

1. merged both branches grid-state-columns and major-release-2
2. pushed to master
3. updated all client-cli and github/doc
   • client-cli throws an error with flatpickr when switching locale, e.g. on Example 12, click on "switch language" and it throws this error:

Error: Module name "flatpickr/dist/l10n/fr.js" has not been loaded yet for context: _

the error comes from this line. Was the error there before? It's possible, but somehow this require doesn't work with loading the locale with RequireJS
4. updated GitHub demo page and also added Example 15 with Grid State & Presets with Local Storage
5. note I did not use the regular conventional-changelog because I wanted to create a Beta tag without doing a Major, however I created a release with few info, you might want to review it.

There's only 1 major thing missing and it's to update the Migration Guide with the new changes you've put in for Editors/Filters. The rest should be covered. Of course we will have to update all the Wikis but that can be progressive.

EDIT
Not sure why NPM doesn't show the Beta version but I tried modifying the package.json of the GitHub demo to download ^2.0.0 and it works. So it's probably just NPM that is delayed.

Let me know when you upgrade your work version, I would like to know if it all works after following the Migration Guide.

I am planning to release the official version in a week or 2, if it all goes well. And as I mentioned in the release, it's in Beta, so if we need to change something, it is still possible to do so. Thanks for all your help 💯

[jmzagorski]

Great job! As for the flatpickr error I never ran into it, but I only use English (however my app will eventually have to be translated into many different languages.)

Do you want me to work on the migration guide for the editor/filter changes I committed?

[ghiscoding]

@jmzagorski
Yes the migration is basically only missing the Editors/Filters changes you made yesterday. The rest should be covered and is well documented, I think

[jmzagorski]

I will work on that. My plan this week is to work on the migration documentation, fix all the unit tests in my app to make sure everything works (my app was also in the process of going from V1 (Kendoui grid) to V2 (your grid)). Then I will close out our long documentation issue and integrate AureliaSlickgrid V2-Beta to see if anything works differently than expected.

[ghiscoding]

Sweet that is a nice plan :)
Hope our lib is better than KendoUI hehe

oh and don't delete the major-release-2 branch, I need it to see the commits you made to port it over my Angular-Slickgrid repo. Thanks

[ghiscoding]

@jmzagorski
I found a new bug that annoys the hell out of me, I spent already half a day on it and I just can't figure out what causes it. The problem is that even if I clear (or change sort) and then I clear of change a filter, sometimes the original sorting comes back!?! I can always reproduce it if I go on Example 4, do a Clear Sorting and right after a Clear Filters, the sorting comes back, wth! From the investigation so far, I found that it comes from this line dataView.refresh() inside the filter callback. Also it puts back the sort icon, only after reaching the callback of that column (basically from the sample below, after triggering a clear filter, it will start showing the sort icon only after reaching the "Duration" column). I have a feeling that it's because of a reference pointer and possibly since our refactoring to support multiple grid.

I don't have an older version anymore, so I can't really test this. Or perhaps, I could use the Github demo and use previous Aurelia-Slickgrid version but I would have to remodify the example 4 again.

[jmzagorski]

Good thing I am behind on my pulls. My master is 9 commits behind, the last commit I have is refactor(demo): update to latest version 1.13.0 and this problem does occur so it is not in our new stuff

[jmzagorski]

Found the issue: the clear filters calls dataView.onRowCountChanged which the sort.service subscribes to here. The args.current from that event call is 478, which is greater than 0 so the local presets are loaded on the grid.

UPDATE:
I will keep my master version where it is in case anything else comes up.

[ghiscoding]

oh ok good thing I asked then, did you just find the issue or do you know how to fix it?

Actually, I think I know what to do, I should simply clear the presets.sorters after clearing the sorting... and that works!!! Thanks man, I was banging my head on this one. I don't think that we need the presets once they are loaded on first page load.

[jmzagorski]

Just found it. Not sure how much work I will be able to do tonight.

[ghiscoding]

ah it's ok, there's no rush. I wanted to push a Beta and it is done. I just encounter coupe of small issues, like that one, while testing other things in my work project. We can now use the master branch again with 2.x going forward, so making small PR is back to the table :)

[ghiscoding]

@jmzagorski
I'm refactoring the Editors to support validators. I didn't know until today that we can watch grid.onValidationError to deal with the result of the validate() function in the Editors and display that validation to the end user.

My question to you is the following, can we pass a custom validator to the editor property in the Grid Definitions? I doesn't work in my Angular repo because I didn't refactor the code that you've done recently with model in editor and filter, but would that work in Aurelia-Slickgrid?

Basically a custom validator would look like this

const myCustomTitleValidator = (value) => {
  if (value == null || value === undefined || !value.length) {
    return { valid: false, msg: 'This is a required field' };
  } else {
    return { valid: true, msg: null };
  }
};

and pass it to the definitions

this.columnDefinitions = [
  { id: 'title', field: 'title', editor: { model: Editors.longText }, validator: myCustomTitleValidator }
];

I didn't try it since I'm not in Aurelia, but I wish it works. Can you confirm?

EDIT
Actually when I started writing it, I thought validator was inside editor but then I realized it was actually inside the Column properties, so yeah it will work.

[ghiscoding]

@jmzagorski
not sure if it's really a good topic or not but I was looking at the implementation of Editors and saw the factory.of on this line. This makes me thing, we could possibly do that also with the formatters to have DI working and get rid of the i18n in the grid options. I think it's only Formatters that aren't DI yet. For example the translateFormatter needs the i18n from grid options, as shown on this line but if we had DI that would fix that formatter.

Hmm actually, our formatters are not classes, not sure if that would implicate a lot more changes on the end user side of it?

[jmzagorski]

Right, formatters are more challenging. Two methods I can think of off the top of my head right now:

1. You would have to define an interface for the user to use so your library knew what function to call on the class after it was instantiated
2. Or the user would have to create a class with their dependencies as the first parameters to their class constructor and then factory.of feeds the rest of the arguments to the constructor, similar to the editor. Then when slickgrid runs it will be calling this Aurelia's invoke every time (not sure what type of overhead that brings)
   For example:

@inject(i18n)
export class MyFormatter {
  constructor(i8n i8n, row, cell, value, columnDef, dataContext) {
  }
}

I am sure there are better ways and I can think of some as I am going through my code.

[ghiscoding]

@jmzagorski
Found a bug with the loading of Filter Presets, if you go on Example 4 you will see the problem. On that example, it's suppose to load search terms "2, 22, 44" but it only loads data with "2". The problem comes from this line, the searchTerms has the 3 values but the operator is empty string and so will do an equal since it's empty, so only first value filter (2) will show up. It's also because this line got removed and so there's no longer any operator defined.

Any suggestions on how to fix this?

I think it should be this

this._columnFilters[columnDef.id] = {
        columnId: columnDef.id,
        columnDef,
        searchTerms,
-        operator: (columnDef && columnDef.filter && columnDef.filter.operator) ? columnDef.filter.operator : null
+        operator: (columnDef && columnDef.filter && columnDef.filter.model.operator) ? columnDef.filter.model.operator : null
      };

actually that seems to cause other problems hmmm

[jmzagorski]

what if you move the creation of the filter here up higher in the code to here so the operator logic can be

operator = columnDef.filter.operator || filter.operator;

Then you can either pass it to updateColumnFilters or add it to the columnDef.filter.operator.

[ghiscoding]

@jmzagorski
Yeah that is a lot better and it simplifies the code too, thanks.

[ghiscoding]

@jmzagorski
oops talked too fast, Example 4 was ok but Example 12 broke since columnDef.filter is undefined. I changed your line to this

operator = filter.operator || (columnDef && columnDef.filter && columnDef.filter.operator);

is that ok?

or the inverse, I don't know which one should be checked first?

operator = (columnDef && columnDef.filter && columnDef.filter.operator) || filter.operator;

[jmzagorski]

that should be fine unless columnDef.filter.operator should take precedence over filter.operator, then you would want to flip the equation.

[ghiscoding]

Yeah I see that in the in ColumnFilter, the operator can be used and maybe defined in the column definition, so I will flip it. Also a filter can be undefined, so I have to take that in consideration too.

So the final result is this

addFilterTemplateToHeaderRow(args: { column: Column; grid: any; node: any }) {
    const columnDef = args.column;
    const columnId = columnDef.id || '';

    if (columnDef && columnId !== 'selector' && columnDef.filterable) {
      let searchTerms: SearchTerm[] | undefined;
      let operator: OperatorString | OperatorType;
+      const filter: Filter | undefined = this.filterFactory.createFilter(args.column.filter);
+      operator = (columnDef && columnDef.filter && columnDef.filter.operator) || (filter && filter.operator) || undefined;

      if (this._columnFilters[columnDef.id]) {
        searchTerms = this._columnFilters[columnDef.id].searchTerms || undefined;
        operator = this._columnFilters[columnDef.id].operator || undefined;
      } else if (columnDef.filter) {
        // when hiding/showing (with Column Picker or Grid Menu), it will try to re-create yet again the filters (since SlickGrid does a re-render)
        // because of that we need to first get searchTerm(s) from the columnFilters (that is what the user last entered)
        searchTerms = columnDef.filter.searchTerms || undefined;
-        this.updateColumnFilters(searchTerms, columnDef);
-        operator = columnDef.filter.operator || undefined;
+        this.updateColumnFilters(searchTerms, columnDef, operator);
      }

I finally took the time to port it over to my Angular repo, which is how I find this issue.

[jmzagorski]

do you need the last || undefined because of typescript?

[ghiscoding]

well I just saw that the filter is const filter: Filter | undefined so if that is the case and it can't find the operator in columnDef, neither in filter, then it will be that 3rd value returned. TS didn't give me any error in my Angular repo but the TSlint is not as strict as it is in my Aurelia repo. I'm quite sure I'll have to do that in Aurelia because of the filter that can be undefined (which will actually never happen but it is declared that way so)

[jmzagorski]

if your first or second expression is false then it will be undefined without the 3rd expression because there will be no 3rd expression to evaluate.

For example if columnDef = undefined and filter = undefined than the result will be undefined with this expression too (columnDef && columnDef.filter && columnDef.filter.operator) || (filter && filter.operator). You can leave it as you have it since the result will be the same, but just thought I would mention that.

[ghiscoding]

oh right I've been working on this for 2 late nights recently and don't see these obvious things anymore lol

[jmzagorski]

lol I have been there too so I know what you are talking about.

[ghiscoding]

@jmzagorski
I have updated the Migration Guide, since I had to do the Angular one too. Now there's only the Wiki to update :P

[ghiscoding]

For reference, the BUG mentioned here is fixed with this commit

The Aurelia-Slickgrid 2.x final will be released early next week. I plan to add Slider Editor this weekend and couple of other changes before being ready for final version.

[jmzagorski]

thanks for updating the migration guide. It looks like the changes I did are in there as well. Sorry I have not been able to do anything. We are missing a few developers so I have to do some extra work over the past week and a half.