Skip to content

fix: range popup and cell popup exist across sheets#6112

Open
KennanChan wants to merge 3 commits into
dream-num:devfrom
KennanChan:fix/popups_binding_to_sheet
Open

fix: range popup and cell popup exist across sheets#6112
KennanChan wants to merge 3 commits into
dream-num:devfrom
KennanChan:fix/popups_binding_to_sheet

Conversation

@KennanChan
Copy link
Copy Markdown
Contributor

@KennanChan KennanChan commented Nov 9, 2025 •
edited
Loading

Description

The range popup and cell popup exists across sheets even with subUnitId specified.

How to test

  • Before

    Filmage.2025-11-09_174508.mp4

  • After

    Filmage.2025-11-09_190218.mp4

Pull Request Checklist

  • Related tickets or issues have been linked in the PR description (or missing issue).
  • Naming convention is followed (do please check it especially when you created new plugins, commands and resources).
  • Unit tests have been added for the changes (if applicable).
  • Breaking changes have been documented (or no breaking changes introduced in this PR).

@KennanChan KennanChan requested a review from jikkai as a code owner November 9, 2025 11:18
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 9, 2025 •
edited
Loading

View Deployment

📑 Demo (React@19) Demo (React@16) 📚 Storybook
🔗 Preview link 🔗 Preview link 🔗 Preview link

@KennanChan
Copy link
Copy Markdown
Contributor Author

KennanChan commented Nov 10, 2025 •
edited
Loading

I don't understand why attachRangePopup and attachPopupToCell allow unitId and subUnitId to be nullable. Since these functions attach popups to ranges or cells, omitting the target sheet doesn't make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jikkai jikkai Awaiting requested review from jikkai jikkai is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@KennanChan