Bugfix/#2867 Fixes popup disappearing in Input Autocompete #2937
Conversation
🦋 Changeset detectedLatest commit: ec52309 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@nullpointerexceptionkek is attempting to deploy a commit to the Skeleton Labs Team on Vercel. A member of the Team first needs to authorize it. |
|
Werid, the test works on my machine
|
|
@nullpointerexceptionkek we typically don't accept drive-by PRs like this. We rather folks discuss the changes upfront to verify they are changes we would accept. In this case, we have instituted a feature freeze on the v2 Popover feature. I'll leave this for now to consider it, but cannot promise it'll move forward. Also, if you would please ahead remove the issue |
Yes, I will make an update on this. So the typical procedure is to discuss in the issue thread? |
Or Discord, etc. Anything so we can ensure folks aren't wasting time on updates we won't accept! We're trying to help in that regard. This is covered in our contribution requirements: https://www.skeleton.dev/docs/contributing/requirements#pull-requests Given it's a small change I may allow it this once, but it'll be lower priority for now. |
|
@nullpointerexceptionkek I'd hoped to review this sooner, but unfortunately the team and I have been swamped with preparing for the upcoming Skeleton v3 release candidate this week. The RC will be the final pre-release version of Skeleton v3 before it merges into the mainline branch. As such, v2 will then enter into an archived state. So I would like to get this PR finalized. The changes as implement in the PR look fine, but I've noted we're seeing a failing unit test as part of the CI checks:
While I don't believe this to be directly related to you change, I wanted to ask two things:
I'd very much like to get your change in before we archive v2. So this is probably the last opportunity to do so. If you can't do this, then we may have forego this one. Thankfully Popovers are implemented in a different manner in v3, so the issue should not carry forward. No pressure, but the next release will be two days from now (Wednesday). If I don't hear from you by then then I will go ahead and close this out. But again, I'd very much appreciate the help! |
|
Hi, Very happy to hear the upcoming release for v3 |
|
It should be good, |
|
@nullpointerexceptionkek everything is green so that works for me. This will be included as part of tomorrow's v2 release. In fact this may even be one of the last public contributions! Thanks again for the quick turn around! |
|
Do you want me to close this one as well? it is a 1 line fix with a important CSS style |
|
I got it, thanks! |
| function handleMouseUp(event: MouseEvent) { | ||
| if (!triggerNode.contains(event.target as Node)) close(); | ||
| } |
There was a problem hiding this comment.
@endigo9740 After a bit of digging, I think this is the root cause for #3075.
There was a problem hiding this comment.
@phamduylong I see. Well I'd welcome a fix if possible. We're only due for about 1 more release of v2 before we archived it, but I'm certain I'm not going to have time to address this directly.
There was a problem hiding this comment.
@nullpointerexceptionkek FYI as well in case you guys want to talk through this.
There was a problem hiding this comment.
I'll take a look at it
There was a problem hiding this comment.
The function handleMouseUp is a fix, or else when the user have an popup open and copies a text outside by dragging the mouse, the popup will not be closed.
There was a problem hiding this comment.
I have PR the changes @phamduylong
Linked Issue
Closes #2867
Description
Fixes by checking if the current selection contains a text.
Changsets
Instructions: Changesets automate our changelog. If you modify files in
/packages, runpnpm changesetin the root of the monorepo, follow the prompts, then commit the markdown file. Changes that add features should beminorwhile chores and bugfixes should bepatch. Please prefix the changeset message withfeat:,bugfix:orchore:.Checklist
Please read and apply all contribution requirements.
devbranch (NEVERmaster)docs/,feat/,chore/,bugfix/pnpm ci:checkpnpm formatpnpm test