Skip to content

fix(popup): dismiss empty create editor on click-away - #788

Open
kduncanhsu wants to merge 1 commit into
box:masterfrom
kduncanhsu:fix/popup-v2-empty-click-away
Open

kduncanhsu wants to merge 1 commit into
box:masterfrom
kduncanhsu:fix/popup-v2-empty-click-away

Conversation

@kduncanhsu

@kduncanhsu kduncanhsu commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Dismiss a staged region, drawing, or highlight when the user clicks away from an empty PopupV2 create editor.
  • Keep the staged annotation and draft when the editor has in-progress text (including mentions). Starting a new mark (annotation) of the same type still replaces the stage.
  • Builds on fix(region): persist staged region annotation on click-away #770: region abort still does not wipe a staged draft; empty dismiss is owned by PopupV2.

Test plan

  • Region: draw a box, leave the comment empty, click the page → region and editor dismiss
  • Region: draw a box, type a comment, click the page → region and comment stay
  • Drawing: add a comment, leave it empty, click the page → drawing and editor dismiss
  • Drawing: type a comment, click the page → drawing and comment stay
  • Highlight: stage a highlight, leave the comment empty, click away (no new selection) → highlight and editor dismiss
  • Highlight: type a comment, click away → highlight and comment stay
  • All types: start a new mark of the same type → previous stage is replaced
  • Click inside the editor or mention UI → does not dismiss

Made with Cursor

Keep the staged annotation and in-progress draft when the user has typed a comment.

Co-authored-by: Cursor <cursoragent@cursor.com>
@kduncanhsu
kduncanhsu requested a review from a team as a code owner September 17, 2026 21:20
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Duncan Hsu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

expect(resetCreator).toHaveBeenCalled();
});

test('should reset creator when PopupV2 cancels a threaded create', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any way to test that reset creator is not called if there is text?

MessageEditorV2: (props: {
isFirstAnnotation?: boolean;
isRichTextEnabled?: boolean;
onChange?: (content: unknown) => void;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any type we can use instead of unknown?

@zhirongwang

Copy link
Copy Markdown
Collaborator

I think it's worth to have another to polish the highlight annotation popup too.

  • highlight some text and popup appear, type some string, click on blank area in the pdf, the pop up disappear.

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.

4 participants