Skip to content

Conversation

cmppoon
Copy link
Contributor

@cmppoon cmppoon commented Aug 18, 2025

Describe Your Changes

Hello, I noticed a couple of issues with the emoji picker in the AddEditAssistant component:

  1. The emoji picker remains stuck in the open state when clicking the emoji icon.

  2. The open state does not reset properly when the dialog is closed with the Escape key. You can see that the picker stays open when trying to add a new assistant.

bug.mov

Fixes Issues

  • Closes #
  • Closes #

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Fixes emoji picker state issues in AddEditAssistant to ensure it closes properly when clicking outside or closing the dialog.

  • Behavior:
    • Fixes emoji picker remaining open when clicking the emoji icon in AddEditAssistant.
    • Ensures emoji picker closes when clicking outside or closing dialog with Escape key.
  • State Management:
    • Adds emojiPickerTriggerRef to track emoji icon clicks.
    • Resets showEmojiPicker state on dialog open/close and form reset.

This description was created by Ellipsis for 9024e2d. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 9024e2d in 1 minute and 11 seconds. Click for details.
  • Reviewed 48 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/dialogs/AddEditAssistant.tsx:63
  • Draft comment:
    Added emojiPickerTriggerRef to hold the reference for the emoji trigger element. This helps differentiate clicks on the picker vs. the trigger.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. web-app/src/containers/dialogs/AddEditAssistant.tsx:69
  • Draft comment:
    The click-outside handler now checks both the emoji picker and its trigger before closing. This prevents the picker from closing if the trigger is clicked.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. web-app/src/containers/dialogs/AddEditAssistant.tsx:97
  • Draft comment:
    Resetting the showEmojiPicker state when loading initial data ensures that the picker does not remain open when reusing the dialog.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
4. web-app/src/containers/dialogs/AddEditAssistant.tsx:127
  • Draft comment:
    Calling setShowEmojiPicker(false) in resetForm ensures that the emoji picker is closed whenever the form is reset.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. web-app/src/containers/dialogs/AddEditAssistant.tsx:251
  • Draft comment:
    Attaching emojiPickerTriggerRef to the emoji icon container ensures that clicks on the trigger do not inadvertently close the emoji picker via the outside click logic.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_siPfRrlwr7W6GFt6

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@urmauur
Copy link
Member

urmauur commented Aug 20, 2025

NICE catch, LGTM 🚀

@urmauur
Copy link
Member

urmauur commented Aug 20, 2025

@cmppoon can u help fix the conflict?

@cmppoon
Copy link
Contributor Author

cmppoon commented Aug 21, 2025

@cmppoon can u help fix the conflict?

@urmauur I have resovled the conflicts.

@louis-menlo louis-menlo merged commit 6c44ec5 into menloresearch:dev Aug 21, 2025
16 checks passed
@github-project-automation github-project-automation bot moved this to QA in Jan Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: QA
Development

Successfully merging this pull request may close these issues.

3 participants