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

[Bug]: adding onCellClick on editor column removes autoEdit , doesn't remove selectEditor #60

Closed
jmzagorski opened this issue May 17, 2018 · 4 comments
Labels

Comments

@jmzagorski
Copy link
Collaborator

I'm submitting a bug report

Current behavior:
in example3.ts if you add an onCellClick to either select editor's column definition (% Complete or Prerequisite), it removes autoEdit and you have to double click to show the editor. Also, the single or multiple select editor stay open when clicking on another cell

Expected/desired behavior:
autoEdit stays enabled and editor goes away when moving to another cell

  • What is the motivation / use case for changing the behavior?
    I am doing advanced filtering in the select editor when the user clicks the cells. I want to change the columnDef.params.collection depending on the dataContext. I have not been able to figure out why these problems occur. Maybe the autoEdit turns off because of this line.

Example of what I am doing. I have a grid of roles below and a column for users. Each row's multipleSelectEditor's collection should be filtered by the role column value (called title in the screenshot) to show the available users. You can also see the problem

image

the logic:

    onCellClick: ({ columnDef, dataContext }) => {
      columnDef.params.collection = this.users.filter(c => c.roles.find(t =>
        dataContext.jobTitle && t.jobTitleName === dataContext.jobTitle.displayName));
    }

@jmzagorski jmzagorski changed the title [Bug]: changing columnDef.params.colection onCellClick removes autoEdit, doesn't remove selectEditor [Bug]: adding onCellClick on editor column removes autoEdit , doesn't remove selectEditor May 17, 2018
@ghiscoding
Copy link
Owner

ghiscoding commented May 17, 2018

You should give a try to the regular onCellClick on the grid object and see if it works there. If that one also doesn't behave correctly there, then it's SlickGrid, else that would be something in gridEvent Service.

With a the grid object

grid.onCellChange.subscribe((e, args) => {
      console.log('onCellChange', args);
      this.updatedObject = args.item;
    });

The example 3 has all the code (in the column or on the grid object), you just need to comment out the portion you don't want.

@jmzagorski
Copy link
Collaborator Author

thanks for that suggestion. Using the grid's onClick event worked (a single clicked opens the editor and the select editor gets removed when clicking another cell). I can use this as a work around and leave this open so we can investigate when we have the time.

@ghiscoding
Copy link
Owner

ghiscoding commented Jun 1, 2018

Fixed in commit on major 2.x branch and cherry picked to master.

The issue is caused by this line e.stopImmediatePropagation(); that is in fact inside the onCellClick. Clicking on the cell, was triggering the stop immediate and stopping the click event bubbling.

I'm thinking about changing the signature of these 2 functions in our new Major release. I basically think that I should re-expose the event (just like SlickGrid does), so that the user can call a e.stopImmediatePropagation(); if he really wants to, without affecting the behavior of the GridEventService

  /** an event that can be used for triggering an action after a cell change */
-  onCellChange?: (args: OnEventArgs) => void;
+  onCellChange?: (e: Event, args: OnEventArgs) => void;

  /** an event that can be used for triggering an action after a cell click */
-  onCellClick?: (args: OnEventArgs) => void;
+  onCellClick?: (e: Event, args: OnEventArgs) => void;

With that new signature, a user could call a stop bubbling from the column definition

this.columnDefinitions = [
 {
      id: 'delete',
      field: 'id',
      formatter: Formatters.deleteIcon,
      onCellClick: (e, args) => {
        this.deleteFromBackend();
        e.stopImmediatePropagation();
     }
  }
];

@ghiscoding
Copy link
Owner

Exposed the Event as first argument in new upcoming 2,x Major version. Closed by commit

ghiscoding added a commit that referenced this issue Jun 5, 2018
* refactor(services): refactor singleton Services into transient
- also add new AureliaGridInstance

* refactor(service): rename GridExtraService to GridService

* deprecate(service): delete GridExtraUtil & move function to GridService

* refactor(code): remove deprecated "onBackendEventApi"

* refactor(code): remove deprecated "selectOptions" from SelectFilter

* refactor(code): remove deprecated FormElementType

* refactor(model): remove exportWithFormatter from the GridOptions

* refactor(service): remove deprecated initOptions replaced by init

* refactor(model): remove deprecated "dataFilters"

* refactor(backend): all backend service methods renamed as processOnX
- to remove confusion with Event Emitters, the 3 Backend Service API methods were renamed to start with the prefix "processOnX" instead of "onX"
- for example onFilterChanged is now processOnFilterChanged

* refactor(gridOptions): all Grid Menu showX were renamed hideX
- since we had both "hideX" (in SlickGrid) and "showX", it's better to rename them all to "hideX" for consistencies

* refactor(events): change aurelia event prefix
- to make a distinction between Aurelia Events vs SlickGrid Events, we will use (asg for Aurelia, sg for SlickGrid)

* refactor(i18n): add i18n Grid Options instead of using params

* refactor(grid): add multiple grids in a view

* refactor(searchTerm): remove searchTerm and only use searchTerms
- prior to this, user could predefined searchTerm (singular) or searchTerms (array). To simplify the logic, the singular searchTerm has been dropped in favor of the array searchTerms

* refactor(example): fix multiple grids not displayed

* refactor(styling): change highlight and selected row color to blueish

* refactor(styling): make selected row a little darker

* refactor(formatter): console error should contain new grid options i18n

* update readme

* refactor(services): add singleton(true) decorator for multiple grids

* fix(i18n): make sure all Services use correct Grid Options i18n

* refactor(example): make row selection titles more obvious

* refactor(event): renamed onGridStateServiceChanged to onGridStateChanged

* feat(grid): expose Slick Grid & DataView objects in AureliaGridInstance

* fix(grid): Dynamically Add Column Header was broken with non-singleton

* refactor(styling): change mouse hover & selection background colors

* refactor(example): add 2x grids in the Basic Grid sample

* refactor(editor): move all Editor params into editor
- instead of using the generic "params" to pass collection and other arguments, we will use the "editor" object
- doing this brings TS types and intellisense

* refactor(service): refactored delete/update item functions

* fix(sorter): issue #72 circular dependency from last PR #70

* fix(event): remove e.stopImmediatePropagation to fix issue #60

* refactor(columnDef): make editor.type the Editor class export function

editor.type was enum, but that means the library would be responsible for finding the editor.
Also, special logic would have to be in place for custom editors.
By making editor.type the exported class function all we have to do is pass it is slickgrid to create

* refactor(example): fixed onCellChange event and advertise delete feature

* refactor(event): expose event to columnDef onCellClick and onCellChange

* refactor(event): use subscriptions array and loop to unsubscribe
- also do the same with Services, make an array and loop through when disposing them

* refactor(editor): change ColumnEditor.type to ColumnEditor.model

* refactor(filter): use Filter model on ColumnFilter, remove FilterType

* feat(gridState): Grid State & Presets for columns (position, size, visibility) (#74)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants