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

Ensure mentions suggestions are displayed when clicking textarea #6730

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Dec 20, 2024

This PR ensures the mention suggestions popover is displayed if clicking the textarea causes the caret to move "into" an at-mention.

Previously this was checked only when the caret's position changed as a result of pressing a key.

Before:

suggestions-on-click-before-2024-12-20_11.58.38.mp4

After:

suggestions-on-click-after-2024-12-20_11.57.50.mp4

Implementation rational

At first I tried to address this by listening to selectionchange event as suggested in this comment, however, it had some problems:

  1. Pressing Backsace does not trigger that event, so we still needed a keyup listener just for that particular key.
  2. The popover has its own listeners to close it on clickaway. I didn't fully debug why, but using selectionchange caused the popover to be closed and then immediately open again.

That's why I ended up leaving the existing keyup listener in combination with click.

@acelaya acelaya requested a review from robertknight December 20, 2024 11:02
@acelaya
Copy link
Contributor Author

acelaya commented Dec 20, 2024

Moving back to draft as I just realized I forgot to add one test case for the new branch. Done

@acelaya acelaya marked this pull request as draft December 20, 2024 11:03
Copy link

codecov bot commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.43%. Comparing base (01d3973) to head (750ab84).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6730   +/-   ##
=======================================
  Coverage   99.42%   99.43%           
=======================================
  Files         270      270           
  Lines       10194    10196    +2     
  Branches     2433     2433           
=======================================
+ Hits        10135    10138    +3     
+ Misses         59       58    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@acelaya acelaya marked this pull request as ready for review December 20, 2024 11:09
src/sidebar/components/MarkdownEditor.tsx Outdated Show resolved Hide resolved
};
// When clicking the textarea it's possible the caret is moved "into" a
// mention, so we check if the popover should be opened
listenerCollection.add(textarea, 'click', checkShouldOpenPopover);
Copy link
Member

Choose a reason for hiding this comment

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

Does this work if you move the caret with long-press on mobile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Let me check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nop, it doesn't 😢. Let me check what's the right event for that.

Copy link
Contributor Author

@acelaya acelaya Dec 20, 2024

Choose a reason for hiding this comment

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

Looks like it's selectionchange. I'm going to create another task to address this once the overall solution is more advanced, as I expect more event handlers to be needed.

I'll adjust on the right combination for different device types and interactions then.

src/sidebar/components/test/MarkdownEditor-test.js Outdated Show resolved Hide resolved
@acelaya acelaya merged commit 82fccad into main Dec 20, 2024
4 checks passed
@acelaya acelaya deleted the mentions-on-click branch December 20, 2024 13:15
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.

2 participants