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

Fix/ min&max width&height on instances #2842

Merged
merged 4 commits into from
Jun 15, 2024
Merged

Conversation

LukeFinch
Copy link
Contributor

Why does this PR exist?

Closes #2426

What does this pull request do?

Changes the logic to apply min and max-width on instances

Testing this change

Create a component with autolayout
Create an instance of the component
Apply min/max width/height sizing through the plugin

Additional Notes (if any)

Copy link

changeset-bot bot commented Jun 11, 2024

⚠️ No Changeset found

Latest commit: 61cb350

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

@LukeFinch LukeFinch changed the base branch from main to release-139 June 11, 2024 10:14
@LukeFinch LukeFinch changed the title Fix/max width on instances Fix/ min&max width&height on instances Jun 11, 2024
@@ -54,8 +53,6 @@ export async function applySizingValuesOnNode(
if (
node.type !== 'DOCUMENT'
&& node.type !== 'PAGE'
&& node.type !== 'INSTANCE'
&& !isPartOfInstance(node.id)
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 needs to stay in - we cant apply min max width tokens on instance children, this would throw an error

CleanShot 2024-06-12 at 10 07 18@2x

Copy link
Collaborator

Choose a reason for hiding this comment

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

as in just the isPartofInstance needs to stay in

Copy link
Contributor

Choose a reason for hiding this comment

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

If this stays, then this is not the fully fledged fix? Can't see any other changes 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

It basically just resolves the case descrbied in the issue: if the layer itself is an instance, we want to allow setting sizing related tokens (as this is possibly). However, if its part of an instance this wouldnt work because figma doesnt allow it.

@robinhoodie0823 robinhoodie0823 requested a review from six7 June 14, 2024 07:38
@six7 six7 merged commit 1a7809e into release-139 Jun 15, 2024
7 of 8 checks passed
@six7 six7 deleted the fix/max-width-on-instances branch June 15, 2024 09:12
@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.

Unable To Apply Max-width Tokens On Component Instances
4 participants