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

DataGrid: Cell Navigation support | CellEdit Mode options #5506

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

David-Moreira
Copy link
Contributor

@David-Moreira David-Moreira commented May 11, 2024

Ok so while improving the cell edit feature I realized the following:

Cell navigation can just be a standalone feature. By activating the feature, both single clicking, using arrow keys & tabbing will navigate through the cells.
I was initially doing it as part of the cell edit mode, but if it's a standalone feature, the user can just activate whenever he wants. We can recommend to activate it in the docs for cell edit mode.

Introduced a new DataGridEditModeOptions. Here we'll have two new options:
CellEditOnSingleClick, CellEditOnDoubleClick that are self explanatory.
Here I would take the breaking change where we default to double click. Users can still enable the old behaviour by setting the CellEditOnSingleClick to true.

Just by adding the extra cell navigation and the single/double click the user has enough flexibility from what I've experienced.

All in all by just adding CellNavigable to a CellEdit enabled Grid the experience is much better!

Let me know your thoughts.

Notes:
You will see that we require a new bit of javascript for this feature, I did try to make it work in C#, but as you know, when you have alot of dynamic elements and you have to try to figure out stuff in DOM it gets really hard and cumbersome. I think it's best to do it in js.

@David-Moreira
Copy link
Contributor Author

Friendly reminder

@David-Moreira David-Moreira requested a review from stsrki May 23, 2024 19:41
@stsrki stsrki changed the title DataGrid | Cell Navigation support | CellEdit Mode options DataGrid: Cell Navigation support | CellEdit Mode options Jun 3, 2024
@mtbayley
Copy link

mtbayley commented Jun 3, 2024

Hi @David-Moreira
I've reviewed this branch and I do like the added navigation features.

However, I am wondering if you are still considering adding these two features I mentioned in my feature request. #5383 (comment). These features would be extremely valuable for my users.

  • Typing any value on the keyboard would put the cell in editing mode.
  • Entering editing mode, selects all text of the current value, allowing a new value to be easily updated

@David-Moreira
Copy link
Contributor Author

Hello @mtbayley
Thanks for reviewing and helping us deliver a better feature.

Both your suggestions seem fine,

  • Typing any value on the keyboard would put the cell in editing mode.
  • Entering editing mode, selects all text of the current value, allowing a new value to be easily updated

Would you agree @stsrki ?
Do you guys think it's worth keeping these behind an optional flag or it's fine if the grid starts working like this?

Also any of you have any suggestions for this feature?
-> "selects all text of the current value"
I don't think I personally ever tried something like this, does blazor support this in some way? I'm guessing not? You guys know the js for this?

@stsrki
Copy link
Collaborator

stsrki commented Jun 4, 2024

Typing any value on the keyboard would put the cell in editing mode.

This seems fine to me. We can do this.

Entering editing mode, selects all text of the current value, allowing a new value to be easily updated

I also don't remember any time doing this. So I would skip it.


The question is how to opt-in. I would add an additional flag to the DataGridEditModeOptions.

@mtbayley
Copy link

mtbayley commented Jun 4, 2024

The question is how to opt-in. I would add an additional flag to the DataGridEditModeOptions.

I agree. Options are good. What might work for me, may not work for someone else.

Also any of you have any suggestions for this feature?
-> "selects all text of the current value"
I don't think I personally ever tried something like this, does blazor support this in some way? I'm guessing not? You guys know the js for this?

I see Blazorise uses the autoNumeric library to select all on focus for the Numeric Picker component.

if (options.selectAllOnFocus.changed) {
newOptions.selectOnFocus = firstNonNull(options.selectAllOnFocus.value, AutoNumeric.options.selectOnFocus.doNotSelect);
}

Can you add something similar to your other input components?

https://github.com/autoNumeric/autoNumeric/blob/ea4a977c230c616fdcda6a5441c6009ce850d2c8/src/AutoNumeric.js#L6421-L6432

_onFocusIn(e) {
    if (this.settings.selectOnFocus) {
        // The whole input content is selected on focus (following the `selectOnFocus` and `selectNumberOnly` options)
        //XXX Firefox <47 does not respect this selection...Oh well.
        this.select();
    } else {
        // Or we decide where to put the caret using the `caretPositionOnFocus` option
        if (!AutoNumericHelper.isNull(this.settings.caretPositionOnFocus)) {
            AutoNumericHelper.setElementSelection(e.target, this._initialCaretPosition(AutoNumericHelper.getElementValue(this.domElement)));
        }
    }
}

https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/select

This is how AG Grid works by default.

msedge_Mb5Ttz4B9L

@David-Moreira
Copy link
Contributor Author

@stsrki Could you evaluate @mtbayley suggestion about the select all focus?
Would probably be best to do it on a separate PR and merge it in after.

@stsrki
Copy link
Collaborator

stsrki commented Jun 12, 2024

We have an API on our input that could be used to select all text. But I would leave that for another PR.

@David-Moreira
Copy link
Contributor Author

We have an API on our input that could be used to select all text. But I would leave that for another PR.

Alright, my point is, if we do that feature for 1.6 or not, and wait for it on this PR?

@stsrki
Copy link
Collaborator

stsrki commented Jun 12, 2024

Do it in another PR. For 1.6.

@mtbayley
Copy link

I appreciate your efforts to get this in 1.6. This will help me out a lot 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow navigating DataGrid with keyboard when in EditMode="DataGridEditMode.Cell"
3 participants