avallete / ag-grid-autocomplete-editor

Quick implementation of autocompletion into ag-Grid cell using autocompleter package.
MIT License
49 stars 25 forks source link

Loss value when you click out of the cell #49

Open nicolagerotto opened 4 years ago

nicolagerotto commented 4 years ago

Hello, I have a strange behavior with the use of the mouse that I found also in the demo.

When a cell is valorized, I enter in editing with the mouse, I do nothing and I click out, the cell loses its value. While if it empties voluntarily it works, but if I don't want it it does the same.

Thanks for your help Nicola

nicolagerotto commented 4 years ago

Hi, any news for this problem?

Regards Nicola

avallete commented 4 years ago

This behaviour have to do with the required option implemented into the isCancelAfterEnd function. I'll investigate it when I have some time.

nicolagerotto commented 4 years ago

Thanks, can we write in private email address? Because I would urgently need to fix it for the release of a project.

Thanks Nicola

avallete commented 4 years ago

If you need a fix ASAP, you should fork the repo and do the fix you need.

I'll be happy to review the PR you make after that when I have the time to do so.

zelinsun commented 4 years ago

You could try "onCellValueChanged" event to set it back to the old value.

nicolagerotto commented 4 years ago

Yes I had already done it, but it is not the desired result. It could be that the cell is actually empty and in that case it's not good. It does not have to assume a value.

avallete commented 4 years ago

Finally got some time to think about your issue.

Returning the problem in all the faces, it always get back of 'what is the desired behaviour of required option'. Actually, his behaviour force the cell to always keep a valid value in it, an empty one, isn't a valid value in that logic.

And I don't see how to change this behaviour without breaking other things with side effects.

In an other hand, for your specific case, doing some tests I came up with this solution:

  {
    headerName: "Already present data selector",
    field: "make",
    cellEditor: AutocompleteSelectCellEditor,
    cellEditorParams: {
      selectData: selectData,
      placeholder: "Select an option",
      // Tell the editor that if he previously had a valid value
      // And that if the edition suddenly stop (eg: click outside the editor, esc press, ...)
      // We want to keep the last valid value in it.
      required: true,
      autocomplete: {
        // Override on select, since we want to check the specific case of 'if input is empty string'
        // In that case, we want to create a 'valid empty value' to allow the `required` option to use
        // This new 'empty' value as a valid one
        onSelect: (cellEditor, item, input) => {
          if (input.value === "" && !item) {
            cellEditor.currentItem = {value: null, label: ""};
          } else if (item) {
            cellEditor.currentItem = item;
          }
        }
      }
    },
    valueFormatter: params => {
      // Simply set the formatter to return empty string if both label and value coers to a false result
      if (params.value) {
        return params.value.label || params.value.value || "";
      }
      return "";
    },
    editable: true
  },

That you can test here.

@nicolagerotto Please let me know if it resolve your use case or not.

ajayupreti commented 4 years ago

I am still facing this issue. I don't want to use the required

avallete commented 4 years ago

@ajayupreti could you please elaborate on why you don't want to use the provided workaround ?

Maybe the workaround discussed here: #57 will suit your use case better ?

You may also try to use the generic ag-Grid ValueSetters and other functions to implement the exact behaviour you want.

PS: I am open to suggestion about how the default behaviour should be, but actually cannot think about another "default" solution since each use-case is pretty specific and I didn't found any proper general formalization of this issue. To me the best way for the package to deal with it is to keep the choice up to the people using it (that's why I expose cellEditor in callback for instance, it give you full control of the behaviour of the cellEditor)

ajayupreti commented 4 years ago

Hi Andrew,

Thanks for your quick reply. The example which you shared https://github.com/avallete/ag-grid-autocomplete-editor/issues/49#issuecomment-565265370 actually I used this piece of code it is clearing value on pressing enter only but not when we clear the value and go out of that cell editor. same issue in the demo https://stackblitz.com/edit/ag-grid-autocomplete-editor-hvx9ev

How to fix that?

avallete commented 4 years ago

Interesting, this it seem related to the way the click events are handled by ag-Grid.

Basically, only Enter and Tab keys make my component know that the "user validated input", or when clicking on a specific item in the list (which my component specifically handle to call "user validated input" callback).

I'll investigate that when I got some free time.

avallete commented 3 years ago

After some investigation it's totally related to the way agGrid handle input. A click outside of the cellEditor is not considered like an "input validation", but like an "input canceled" instead.

This might be overridden into autocomplete editor by catching the click event before ag-grid and stopping this propagation. Then triggering the 'input validated' event instead of the 'input canceled' event. Leaving only one way to cancel input (a hit on the Esc key).

Need first to streghten the e2e "natives" tests with cypress-real-events (#93). Then I might introduce this behavior in an option without breaking something else.