Skip to content

feat: fix bounding bug on NCweb - #149

Merged
kirillgroshkov merged 1 commit into
mainfrom
red-dot-nc-web
Jul 2, 2026
Merged

feat: fix bounding bug on NCweb#149
kirillgroshkov merged 1 commit into
mainfrom
red-dot-nc-web

Conversation

@dgozen

@dgozen dgozen commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

There is a bug on NCWeb where the cntrl shift L bugs out and can not be properly opened and closed. I made a quick fix on NCWeb in this branch: https://github.com/NaturalCycles/NCWeb/pull/4704 but thought I could fix it here...

In dev, React StrictMode runs each effect twice on purpose on mount, cleanup, mount again to catch bugs like this one. Because cleanup couldn't remove the first listener, we ended up with two listeners on the page. I could see that every keypress fired both when console logging one flipped adminMode on, the other immediately flipped it back off, so nothing visibly changed. Production doesn't do this double thing with mount/cleanup, so only one listener was added and worked as expected

Dont know who to tag for this...

@dgozen
dgozen requested a review from kirillgroshkov as a code owner July 2, 2026 10:15
@dgozen

dgozen commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@matteo-tonini mentioned that the NCWeb PR might be enough to fix it so maybe this is not necessary 🤷‍♀️ Happy to close otherwise

@kirillgroshkov kirillgroshkov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, makes sense 👍

@kirillgroshkov
kirillgroshkov merged commit 54139ed into main Jul 2, 2026
3 checks passed
@kirillgroshkov
kirillgroshkov deleted the red-dot-nc-web branch July 2, 2026 12:53
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

🎉 This PR is included in version @naturalcycles/js-lib-v15.79.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants