search

tokens-studio / figma-plugin

Official repository of the plugin 'Tokens Studio for Figma' (Figma Tokens)
https://www.figma.com/community/plugin/843461159747178978
MIT License
1.32k stars 190 forks source link

Fix for clicking outside open Select closing modal #2919

Closed macintoshhelper closed 1 week ago

macintoshhelper commented 1 week ago

Why does this PR exist?

Closes #2885

What does this pull request do?

Testing this change

Additional Notes (if any)

For testing locally, it is necessary to clear node_modules and do a clean yarn install.

In the repository root directory:

rm -rf node_modules
yarn

https://github.com/tokens-studio/figma-plugin/assets/6757532/33cfdc49-fa40-42bb-bacb-6a264be804d3

changeset-bot[bot] commented 1 week ago

⚠️ No Changeset found

Latest commit: e4345e58410915ceb4bfd4a48e8cd9bdf8c2c9fc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

cuserox commented 1 week ago

I'm wary of other multiple versions that have appeared in the yarn.lock file, such as:

Do you think this is due to the pinned dependency of "@radix-ui/react-dismissable-layer": "1.0.5"?

If so, this should be carefully tested beforehand. Next option on the table is to upgrade all linked up radix-ui packages (Dialog & Select) and hope the latest versions work together nicely.

What does the team think? CC @macintoshhelper @six7 @robinhoodie0823

For reference, my previously closed PR was another 'fix' option which involved manipulating the DOM, the caveat was that the Select lost its hover / pointer effects: https://github.com/tokens-studio/figma-plugin/pull/2892

Depends on the annoyance of the users as per the initial report (linked here and @SamIam4Hyma 🙏)

six7 commented 1 week ago

I'm wary of other multiple versions that have appeared in the yarn.lock file, such as:

Do you think this is due to the pinned dependency of "@radix-ui/react-dismissable-layer": "1.0.5"?

If so, this should be carefully tested beforehand. Next option on the table is to upgrade all linked up radix-ui packages (Dialog & Select) and hope the latest versions work together nicely.

What does the team think? CC @macintoshhelper @six7 @robinhoodie0823

For reference, my previously closed PR was another 'fix' option which involved manipulating the DOM, the caveat was that the Select lost its hover / pointer effects: #2892

Depends on the annoyance of the users as per the initial report (linked here and @SamIam4Hyma 🙏)

@macintoshhelper can you do a check of various modals / dropdowns in the plugin to ensure that we didnt regress? if so i'd say that'd be enough of a check given we'd want to upgrade radix packages soon.

otherwise 👍 from my side