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

[HxInputTags] Allow Enter key to confirm the tag #585

Closed
TPIvan opened this issue Aug 24, 2023 · 7 comments
Closed

[HxInputTags] Allow Enter key to confirm the tag #585

TPIvan opened this issue Aug 24, 2023 · 7 comments
Labels
enhancement New feature or request up for grabs

Comments

@TPIvan
Copy link
Contributor

TPIvan commented Aug 24, 2023

I am using HxInputTags and I want to enable the users to enter tags from a predefined list or type their own texts. However, I found that the users often expect the Enter key to confirm the tag, but it does not work. Some of them manage to confirm the tag by clicking outside the component area, but none of them try to use the space or comma keys. I have tried to use the Delimiters attribute to include the Enter key as a delimiter, like this ( '!' is added to test that the attribute is applied):

Delimiters="new List<char> () {'\x21'/*!*/, '\x0D'/*CR*/ , '\x0A'/*LF*/ }"

But it does not seem to have desired effect. Is there a way to make the Enter key work as a delimiter for HxInputTags? I think this would improve the user experience and avoid confusion. Thank you for your assistance.

@hakenr
Copy link
Member

hakenr commented Sep 25, 2023

The Enter key was deliberately excluded from confirming the tag in the current implementation. After discussing the expected behavior within the team, we've decided to change this approach. Pressing Enter will confirm the new tag, regardless of whether it originates from the suggestions or is a completely new tag to add. This action will not be treated as a delimiter and will not be configurable through the Delimiters parameter, similar to how the current Tab key functions.

Note to implementers: Careful not to submit the form with such action, see #584.

@hakenr hakenr added this to the Priority 3 - Low milestone Sep 25, 2023
@TPIvan
Copy link
Contributor Author

TPIvan commented Nov 15, 2023

I have come up with a possible solution and I would like to create a PR if you approve it.

In HxInputTagsAutosuggestInputInternal.razor, I added a HandleKeyDown method to handle the onkeydown event of the input element.

 @onkeydown="HandleKeyDown"

	public async Task HandleKeyDown(KeyboardEventArgs e)
	{
		if (e.Code == "Enter" || e.Code == "NumpadEnter")
		{
			await OnEnter.InvokeAsync();
		}
	}

 [Parameter] public EventCallback OnEnter { get; set; }

The OnEnter parameter is used in HxInputTagsInternal.razor to invoke the HandleEnter method, which processes the custom tags when the Enter key is pressed.

 private async Task HandleEnter()
 {
	if (!AllowCustomTags) { return; }

	if (!isDropdownOpened)
	{
		await TryProcessCustomTagsAsync(false);
		userInput = String.Empty;
		StateHasChanged();
	}
 }

I have some questions regarding this solution:

  1. Do you think we should provide an option for the users to enable or disable this feature (e.g. AllowEnterConfirm)?
  2. Should we add a stopPropagation attribute to the onkeydown event handler to prevent the event from triggering other actions on the parent elements?
  3. I copied a part of the HandleEnter method (the if (!isDropdownOpened) block) from the OnAfterRenderAsync method, because I did not want to change the existing code. Do you think we should extract this logic into a separate method to avoid code duplication?

@hakenr
Copy link
Member

hakenr commented Nov 25, 2023

@TPIvan, thank you for your eagerness to contribute.

Regarding subscribing to @onkeydown in Blazor, I suggest we avoid it unless it's absolutely necessary, considering performance reasons. We're already using @oninput, which results in the component communicating with Blazor twice for most keystrokes. Our goal should be to devise a solution that minimizes these Blazor roundtrips and efficiently handles most scenarios using HTML/JavaScript.

Addressing your questions:

  1. Should we offer a toggle for users to enable/disable this feature (e.g., AllowEnterConfirm)?
    I don't believe that's necessary.

  2. Should we include a stopPropagation attribute in the onkeydown event handler to prevent the event from affecting parent elements?
    This should be conditional. It's probably best to apply it only to keystrokes that the component manages. We want to avoid interfering with global keyboard shortcuts.

  3. Regarding the duplicated logic in HandleEnter method (from the if (!isDropdownOpened) block in OnAfterRenderAsync), should we extract this into a separate method?
    Absolutely, we should abstract this logic into a separate method to prevent code duplication.

@TPIvan
Copy link
Contributor Author

TPIvan commented Nov 27, 2023

@hakenr, thank you for your answer.

I understand your concern about avoiding handling @keydown/@keypress in Blazor for performance reasons. The problem is that @oninput event does not fire for the enter key, so we cannot rely on it to trigger the tag creation.
The only alternative solution I can think of is to use JavaScript to handle and filter the keydown/keypress events and create a custom event for OnEnter. I am not very familiar with it and I am not sure if it is worth the effort. To me it seems to be a bit too complex.

@hakenr
Copy link
Member

hakenr commented Nov 27, 2023

No problem, we will handle this ticket on our own.
But the current priority is Low, unfortunately.

@TPIvan
Copy link
Contributor Author

TPIvan commented Nov 27, 2023

I wanted to know whether you consider the solution with @onkeydown completely unacceptable. In that case I will still consider doing it but also with lower priority.
My proposal would be to make it with blazor key handling and toggling property (AllowEnterConfirm) and leave more advanced event handling as a future performance enhancement if needed.

@hakenr
Copy link
Member

hakenr commented Dec 30, 2023

In the end, the HxInputTags component already manages all the keydown events, and I incorporated handling for the Enter key into the existing logic.

With all the other fixes and enhancements I've implemented recently, it has become a significant overhaul of the HxInputTags component. The new implementation requires thorough testing.

@hakenr hakenr closed this as completed in 5891820 Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request up for grabs
Projects
None yet
Development

No branches or pull requests

2 participants