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

OnKeyDown in Modal #4987

Open
NCBR opened this issue Sep 8, 2023 · 8 comments
Open

OnKeyDown in Modal #4987

NCBR opened this issue Sep 8, 2023 · 8 comments
Assignees
Labels
Type: Possible Bug Needs to investigate more to see if it's an actual bug.

Comments

@NCBR
Copy link

NCBR commented Sep 8, 2023

Hey, I've got a bit of an issue here

So I have a ModalService set up, that opens a custom component. That all works fine.
My issue is that when I have ANY OnKeyDown or OnKeyUp events in any of the child content, if escape is pressed it will close the modal. Without any OnKeyDown or OnKeyUp in the child content, pressing escape actually does nothing, doesn't even trigger the closing sequence.

I have an OnKeyDown on my datagrid, which I want to close the row editor if it is in Edit or New state when the Escape button is pressed. What happens instead is that the entire modal closes. I would like to be able to use stopPropagation so I can stop it from continuing out further, but that causes the website to become unresponsive (can't have two OnKeyDown).

I do still want to close the modal if Escape is pressed while not focusing the datagrid.

It seems to me that there is some issue with the handling of the closing sequence, as the modal closing sequence doesn't trigger for me without a OnKeyDown/OnKeyUp sequence present, whether it actually is meant to close it. Any old one will do apparently.

I know I can cancel the closing sequence, but that is also not going to solve my specific issue, as I still want it to close with Escape, just not by Escape being pressed while the datagrid is selected.

Edit: I may be able to provide some code snippets if needed, but due to it being a project for work I do have some limitations in what I can show.

@David-Moreira
Copy link
Contributor

Hello,
Please provide a simple reproducable.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

Hello @NCBR, thank you for your submission. The issue was labeled "Status: Repro Missing", as you have not provided a way to reproduce the issue quickly. Most problems already solve themselves when isolated, but we would like you to provide us with a reproducible code to make it easier to investigate a possible bug.

@NCBR
Copy link
Author

NCBR commented Sep 8, 2023

DataEntryModalExample.razor
@ using System.ComponentModel.DataAnnotations;

Test Add New Value
<DataGrid @ref="@_dataGrid" TItem="ExampleEntry"
          Data="@_entries"
          @bind-SelectedRow="@_selectedEntry"
          Responsive Editable UseValidation SubmitFormOnEnter Navigable @onkeydown="@(async (e) => await CancelRow(e))">
    <DataGridColumns>
        <DataGridColumn Sortable Field="@nameof(ExampleEntry.Value)" Caption="Value" Editable DisplayFormat="{0:0.####}" VerticalAlignment="VerticalAlignment.Middle" DisplayFormatProvider="@System.Globalization.CultureInfo.GetCultureInfo("da-DK")">
            <EditTemplate>
                <NumericPicker @ref="@_numericPicker" TValue="float?" Value="@((float?)context.CellValue)" AlternativeDecimalSeparator="," Decimals="4" Min="0.0000f"  Max="10000.0000f" RoundingMethod="NumericRoundingMethod.HalfEvenBankersRounding" ValueChanged="@( v => context.CellValue = v)"  Autofocus SelectAllOnFocus/>
            </EditTemplate>
            <DisplayTemplate>
                @{
                    @($"{String.Format(CultureInfo.CreateSpecificCulture("da-DK"), "{0:0.00##}", context.Value)}")
                }
            </DisplayTemplate>
        </DataGridColumn>
    </DataGridColumns>
</DataGrid>

@ code {
[Inject] public IModalService ModalService { get; set; }

private List<ExampleEntry> _entries = new List<ExampleEntry>();

private ExampleEntry _selectedEntry;

private DataGrid<ExampleEntry> _dataGrid;
private NumericPicker<float?> _numericPicker;

private async Task StartNew()
{
    await _dataGrid.New();
}

protected override void OnInitialized()
{
    this.ModalService.ModalProvider.Closing = OnModalClosing;
}

private async Task Confirm()
{
    if (_dataGrid.EditState == DataGridEditState.None)
    {
        await ModalService.Hide();
    }
}

public async Task CancelRow(KeyboardEventArgs e)
{
    if (!e.Repeat && e.Code == "Escape" && _dataGrid.EditState != DataGridEditState.None)
    {
        await _dataGrid.Cancel();
    }
}

private Task OnModalClosing(ModalClosingEventArgs e)
{
    if (e.CloseReason == CloseReason.FocusLostClosing || e.CloseReason == CloseReason.None || e.CloseReason == CloseReason.EscapeClosing)
    {
        if (_dataGrid.EditState != DataGridEditState.None)
        {
            e.Cancel = true;
        }
    }

    return Task.CompletedTask;
}

public class ExampleEntry
{
    [Required]
    [Range(0.0000f, 10000.0000f)]
    public float? Value { get; set; }
}

}

DashboardModalExample.razor
@ page "/testmodal"

Open Modal

@ code {
[Inject] public IModalService ModalService { get; set; }

public Task ShowModal()
{
    return ModalService.Show<DataEntryModalExample>(x =>
    {
    },
    new ModalInstanceOptions()
        {
            UseModalStructure = false,
            Size = ModalSize.ExtraLarge,
            FocusTrap = true
        });
}

}

When datagrid is in new/edit state here, it cancels. But if you put an onkeydown event for escape key on the datagrid that then cancels the line, and subsequently closes the entire modal, instead of only cancelling the line.

@David-Moreira David-Moreira added Type: Possible Bug Needs to investigate more to see if it's an actual bug. and removed Status: Repro Missing labels Sep 9, 2023
@David-Moreira
Copy link
Contributor

Hello @NCBR
I just tried your example. Seems like a race condition. Stuff get's executed asynchronously, and the CancelRow actually ends up executing first, so the DataGrid won't be on an Edit State when OnModalClosing is called.

Maybe try adding a flag to figure out if the row was already cancelled? Can you try something like this?

    public async Task CancelRow( KeyboardEventArgs e )
    {

        if (!e.Repeat && e.Code == "Escape" && _dataGrid.EditState != DataGridEditState.None)
        {
            _cancelledRow = true;
            await _dataGrid.Cancel();
        }
    }

    private bool _cancelledRow = false;
    private Task OnModalClosing( ModalClosingEventArgs e )
    {
        if (e.CloseReason == CloseReason.FocusLostClosing || e.CloseReason == CloseReason.None || e.CloseReason == CloseReason.EscapeClosing)
        {
            if (_dataGrid.EditState != DataGridEditState.None || _cancelledRow)
            {
                e.Cancel = true;
                _cancelledRow = false;
            }
        }

        return Task.CompletedTask;
    }

@NCBR
Copy link
Author

NCBR commented Sep 12, 2023

I tried your suggestion and it did work. Now the only problem is if I click out of the datagrid with my mouse (so I click anywhere on the modal), the modal doesn't seem to respond to escape being pressed.

If I press escape while the datagrid is selected, it correctly cancels the row. I can then press escape again to close the modal.

But if I click anywhere on the modal, it does not respond to escape until I select the datagrid again.

Honestly not sure why it does that, or whether it's just on my end.

@David-Moreira
Copy link
Contributor

Glad it worked.
As for your current issue, I suspect, It might be because it thinks the focus is elsewhere and the escape is not triggered on the modal itself. But we'll have to take a look to make sure.

We'll let you know once we have news again.

@stsrki
Copy link
Collaborator

stsrki commented Jul 30, 2024

@David-Moreira, should we close this, or do you think we can add some extra logic to the codebase that handles this use case?

@David-Moreira
Copy link
Contributor

We can take a look just to be sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Possible Bug Needs to investigate more to see if it's an actual bug.
Projects
None yet
Development

No branches or pull requests

3 participants