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

fix: Enter Key Functionality in the Response Editor when @ symbol is … #10699

Conversation

iamsmruti
Copy link
Contributor

@iamsmruti iamsmruti commented Jan 16, 2025

Description

Fixes #10698
This MR addresses a bug in the Enter key functionality when typing "@" symbols followed by text in the editor. Specifically:

  • Fixed the behavior where hitting Enter does not move to the next line when "@" and the following text share the same style (e.g., both bold or italic).
  • Ensured that Enter key functionality correctly transitions between user selection and new line insertion based on whether a popup suggestion is available.
  • Added logic to handle scenarios where no matches are found in the suggestion popup.

Demo

Screen.Recording.2025-01-17.at.1.19.16.AM.mov

Testing Scenarios

Please validate the following scenarios to ensure the fix is comprehensive:

  • Scenario 1: Type "@" and select a username from the popup. Verify that hitting Enter selects the user as expected.

    • Step 1: Type "@"
    • Step 2: Start typing a username and select it from the popup.
    • Step 3: Hit Enter and ensure the user is inserted correctly.
  • Scenario 2: Type "@" followed by text with no matching user and hit Enter. Ensure the cursor moves to the next line.

    • Step 1: Type "@nonexistent-text"
    • Step 2: Hit Enter and confirm the new line is created.
  • Scenario 3: Validate that no regressions occur with existing mention functionality:

    • Ensure the suggestion popup works as expected.
    • Ensure the popup hides when Escape is pressed.

Final checklist

  • I checked the code review guidelines
  • I have added Metrics Representative as reviewer(s) if my PR invovles metrics/data/analytics related changes
  • I have performed a self-review of my code, the same way I'd do it for any other team member
  • I have tested all cases I listed in the testing scenarios and I haven't found any issues or regressions
  • Whenever I took a non-obvious choice I added a comment explaining why I did it this way
  • I added the label Skip Maintainer Review Indicating the PR only requires reviewer review and can be merged right after it's approved if the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'
  • PR title is human readable and could be used in changelog

@iamsmruti
Copy link
Contributor Author

Hi @Dschoordsch, please review this PR

@iamsmruti
Copy link
Contributor Author

@Dschoordsch Please have a look at this PR

@Dschoordsch Dschoordsch self-requested a review January 20, 2025 18:51
component?.destroy()
if (props.event.key === 'Enter') {
if (teamMembersFiltered.length === 0) {
return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 I think you were on the right path here and I wouldn't have found it that quickly without this, but I think we shouldn't modify this component. Keeping a reference to the filtered elements also feels a bit hacky. Instead I think the error lies in the component, namely the enterHandler in MentionDropdown. We should check there if the selectedIndex is out of bounds of items and return false. In the onKeyDown handler we have to return that value. This way we can get away without keeping the additional reference to the teamMembersFiltered

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right, I will give it a try

@iamsmruti iamsmruti force-pushed the fix/enter-key-issue-at-symbol-styling-in-response-editor branch from 86cf426 to f098784 Compare January 23, 2025 06:02
@iamsmruti
Copy link
Contributor Author

@Dschoordsch Yes, adding the checks in the enterHandler keeps things, clean. Thanks for the suggestion. I have made the changes. Please review it again.

Let me know if I am missing anything.

@Dschoordsch Dschoordsch self-requested a review January 23, 2025 10:32
@Dschoordsch
Copy link
Contributor

Please re-request review next time you need another pass (the refresh button next to the reviewer name), otherwise it might get forgotten.

@Dschoordsch Dschoordsch merged commit cd3506d into ParabolInc:master Jan 23, 2025
4 checks passed
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.

Bug: Enter Key Not Functioning Properly with "@" Symbol in the Response Editor
2 participants