Prevent cell content removal on click (when editor is used)

[manju-reddys]

Hello,

I have custom editor for cell, the problem i'm facing is whenever the cell is clicked the existing content (DOM) is getting removed and also I'm not able to listen to the events registered to the cell content DOM. Is there way I can prevent the cell content removed by default? how do I listen to the cell content DOM events?

In the attached image, the 2 input boxes must show on clicking "Set Range" and if user click on the clear icon (red x circled) the cell content should set back to "Set Range". I have implemented using "grid.onClick.subscribe" but this have other challenges like validation. I couldn't use the editor because of the cell content getting cleared by default.

[6pac]

From your description of the problem, I suspect that you don't understand how the editors work.
Check out: example3a-compound-editors.html
You need to implement the editor separately from the formatter and then specify it in the column definition. The formatter result is cleared and the editor is created when you click on the cell - the formatter and editor are completely separate things and one should not activate the other.

I have embedded full jQueryUI controls as editors, so there's no problem with complex editor contents.

[manju-reddys]

Well, if my description is not clear I will explain here again:

As in attached image, I have requirement to clear the inputs (range) only when user click on the clear button (red circled).

I understood the concept of formatter (how to render the value) and the editor (what to do on click), the problem what I'm talking about is how to prevent the content getting clear on click?. Before the editor init function get called the content is getting cleared.

To be more specific in the slick.grid.js => Method "makeActiveCellEditable" the condition

// don't clear the cell if a custom editor is passed through
            if (!editor) {
                activeCellNode.innerHTML = "";
            }

How to set the parameter editor to true, so the content will be prevented to clear on click of the cell before the init method in the editor get called?

[6pac]

What I mean is that to keep it simple, you should write a formatter/editor pair that conforms to the normal behaviour of the grid editors. See example3a-compound-editors.html for a perfectly good range editor.
However, if you must do what you are trying to do, that requires a workaround of the default behaviour, and I think changes to the core code.

function handleClick(e) {
  if (!currentEditor) {
    // if this click resulted in some cell child node getting focus,
    // don't steal it back - keyboard events will still bubble up
    // IE9+ seems to default DIVs to tabIndex=0 instead of -1, so check for cell clicks directly.
    if (e.target != document.activeElement || $(e.target).hasClass("slick-cell")) {
      setFocus();
    }
  }

  var cell = getCellFromEvent(e);
  if (!cell || (currentEditor !== null && activeRow == cell.row && activeCell == cell.cell)) {
    return;
  }

  trigger(self.onClick, {row: cell.row, cell: cell.cell}, e);
  if (e.isImmediatePropagationStopped()) {
    return;
  }

  if ((activeCell != cell.cell || activeRow != cell.row) && canCellBeActive(cell.row, cell.cell)) {
    if (!getEditorLock().isActive() || getEditorLock().commitCurrentEdit()) {
      scrollRowIntoView(cell.row, false);
      setActiveCellInternal(getCellNode(cell.row, cell.cell));
    }
  }
}

by setting isImmediatePropagationStopped in the click event handler, you can then write a customised version of the final block. The problem is that while makeActiveCellEditable allows you to pass a custom editor object, setActiveCellInternal (which calls makeActiveCellEditable) doesn't. So we'd have to either mod setActiveCellInternal or create some custom editor setting to allow suppressing of the cell clear.
It's difficult, and I'll have a think on the best way to do it.
I agree that it should be possible without modding core code. But it's a lot easier just to follow the existing behaviour.

[6pac]

OK, I've decided the best way to do this is to add a property suppressClearOnEdit to the editor itself. eg:

function TextEditor(args) {
  var $input;
  var defaultValue;
  var scope = this;
  this.suppressClearOnEdit = true;

  this.init = function () {

edit: see comments below, the above is WRONG and should be:

function TextEditor(args) {
  var $input;
  var defaultValue;
  var scope = this;

  this.init = function () {
...

  this.init();
}

TextEditor.suppressClearOnEdit = true;

I've then made makeActiveCellEditable check for that property:

  $(activeCellNode).addClass("editable");

  var useEditor = editor || getEditor(activeRow, activeCell);

  // don't clear the cell if a custom editor is passed through
  if (!editor && !useEditor.suppressClearOnEdit) {
    activeCellNode.innerHTML = "";
  }

  currentEditor = new useEditor({

These changes have just been added to the repo here. Let me know if it works for you.

[manju-reddys]

This will not work because the variable "suppressClearOnEdit" is a property of the editor function and we can get access to it only if we call (new instance of) the function. At this point the we haven't called the editor method so it will be undefined (not exposed).

The simplest and easiest way to solve this problem is by add property ("suppressClearOnEdit") to the column definition rather to trying to access the method property.

[6pac]

Sorry, you are correct that the code as-is will not work. My bad.
The property needs to be assigned after the editor function, eg:

function TextEditor(args) {
  var $input;
  var defaultValue;
  var scope = this;

  this.init = function () {
...

  this.init();
}

TextEditor.suppressClearOnEdit = true;

I did consider the column property, but I still think this is a better approach, because the behaviour is part of the editor, and might be used in multiple columns, or columns might have editors dynamically reassigned.

[manju-reddys]

I don't see the side effects of adding a variable to the column definition, its more flexible then your approach. Your suggestion will fails if we define the editor as anonymous function.

[6pac]

I just think philosophically the behaviour is part of the editor, not the column. The only connection with the column is that the editor happens to have been assigned to it, and I want to avoid creating more 'noise' in the column defaults if at all possible.

It would fail if the editor were anonymous, but why would you want to do that ?

[manju-reddys]

In my opinion: Its not part of editor but column definition, why? answer is simple, the editor is reusable component, the same editor can be in more than one column but each column may have different behaviors. It is not always true that the default operation (empty the cell) to be prevented in all columns the editor is used. So I believe that adding it as column definition (config) is the best solution.

In my case, I use anonymous function (as editor) in case if no reusable code and not required complex logic. Like simple inline editing (just example - We can use the one comes with editor but can't customize it to our needs).

Unless a proper solution closing the ticket is not a good sign.

[6pac]

Look I'm sorry but I don't agree with any of these points.
The principle is that editors are a reusable plugin. As such, they should be named, live in a library file, and have consistent behaviour including clearing the cell.
You are welcome to change your own code, that is a trivial change, but in my opinion it is the role of the grid architecture to encourage good practices and provide a workable and consistent API.

So again, sorry, this is not a problem I have with you. As the gatekeeper for this project, consistency and simplicity are really important as they affect maintainability of the code, especially in team environments. It's easy to let requirements for flexibility turn the API into a hotchpotch of methods, when it's often better to make things slightly less flexible but architected in a way that promotes consistent program structure.
Those are the considerations I'm following, and I really think the solution provided is the best one for the main grid trunk. That's really all I have to say.

[manju-reddys]

That is okay and thanks, I have already forked to go my way.