Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2837 20 unable to create variable collections from sets #2844

Merged

Conversation

robinhoodie0823
Copy link
Contributor

Why does this PR exist?

Current plugin does not create local variable collections based on selected token sets. Because the createNecessaryVariableCollections function does not accept the parameter structure in case of selected sets.

Closes #2837

What does this pull request do?

Updated createLocalVariablesWithoutModesInPlugin function to send parameters with the structure which createNecessaryVariableCollections function can accept to proceed.

Testing this change

Additional Notes (if any)

20240611_203716.mp4

Copy link

changeset-bot bot commented Jun 12, 2024

⚠️ No Changeset found

Latest commit: f743f4a

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

@robinhoodie0823 robinhoodie0823 requested a review from six7 June 12, 2024 00:37
@robinhoodie0823 robinhoodie0823 linked an issue Jun 12, 2024 that may be closed by this pull request
@@ -30,6 +30,18 @@ export default async function createLocalVariablesWithoutModesInPlugin(tokens: R

const checkSetting = !settings.variablesBoolean && !settings.variablesColor && !settings.variablesNumber && !settings.variablesString;
if (!checkSetting) {
const themesToCreateCollections = selectedSets.reduce((acc: ThemeObject[], curr: ExportTokenSet) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

would you not be able to re-use the themeContainer const on line 45 for this? I think thats what it was intended for?

const { collection, modeId } = findCollectionAndModeIdForTheme(set.set, set.set, collections);

if (!collection || !modeId) return;

const allVariableObj = await updateVariables({
collection, mode: modeId, theme: themeContainer, tokens: setTokens, settings, filterByTokenSet: set.set,
collection, mode: modeId, theme: themeContainer, tokens, settings, filterByTokenSet: set.set,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this part here is whats causing an issue when multiple sets have the same tokens. For example, load up the Preset, and then activate each set. Try to create variables based on sets. Notice that the light set is going to be empty.

image

@robinhoodie0823 robinhoodie0823 requested a review from six7 June 12, 2024 08:58
@six7 six7 merged commit 587f7a0 into release-139 Jun 12, 2024
7 of 8 checks passed
@six7 six7 deleted the 2837-20-unable-to-create-variable-collections-from-sets branch June 12, 2024 20:03
@six7 six7 added the 2.0-rc8 label Jun 16, 2024
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.0: Unable to create variable collections From Sets
2 participants