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

can you support flex-wrap and hug content( without fixed width and height) #82

Open
kaneruan opened this issue Aug 21, 2023 · 20 comments
Open

Comments

@kaneruan
Copy link

can you support flex-wrap and hug content( without fixed width and height)

@bernaferrari
Copy link
Owner

Yes, I just need to figure out the new API.

@davestewart
Copy link
Contributor

Hey, have the same issue here.

Seems like this line could just be:

  if (typeof size.width === "number" && (node as FrameNode).layoutSizingHorizontal === "FIXED") {

CleanShot 2024-07-02 at 17 21 27

@bernaferrari
Copy link
Owner

if it works well feel free to make a PR :P but it should be "layoutSizingHorizontal" in node && node.layoutSizingHorizontal, FrameNode is too restrictive. InstanceNode, ComponentNode, other things might have it too.

@davestewart
Copy link
Contributor

davestewart commented Jul 3, 2024

Hey @bernaferrari, @kaneruan ...

I'm looking at this, this morning, and continuing the discussion here (vs the previous PR) so others can contribute.

It seems that to better-support Flexbox and Tailwind that some more work is going to need to be done.

Flexbox / sizing

Currently the HTML and Tailwind exports do not correctly support the various fixed/fill combinations.

Here, the boxes have fixed widths:

CleanShot 2024-07-03 at 10 39 14

Yet, they export with flexbox as scaled to fit (normal flexbox behaviour as I understand it):

CleanShot 2024-07-03 at 10 35 05

They should only do this if set to fill container:

CleanShot 2024-07-03 at 10 39 28

Setting shrink-0 would solve this (may also need to check parent resizing mode, not sure yet):

CleanShot 2024-07-03 at 10 30 47

And would also work in Tailwind:

https://play.tailwindcss.com/6I3tJVO5vv

CleanShot 2024-07-04 at 12 32 09

Flexbox / axis

To achieve wrapping, I'll need to tweak the align-items / align-content CSS properties when wrapping is on, but I seem to have this working:

CleanShot 2024-07-03 at 10 54 48

In summary

If everyone is happy with this, I'll implement the following:

  • shrink-0 for fixed-size content (not needed when wrapping)
  • align-items / align-content for wrapped content

Obviously flexbox is a fairly complex beast so I'll do my best to build on top of what has already been done, but also open to input / tips as I start to write code.

LMK, thanks.

@davestewart
Copy link
Contributor

@bernaferrari – what is the optimizeLayout flag supposed to do?

It seems to just prefer inferredAutoLayout.* to node.* in the functions it's used it.

I see this is a user checkbox, vs something used internally.

I understand that the property is a kind of amalgamation of node properties. I've run some test code and it appears that the two are always in sync.

Does it make a difference to the code output?

Am I missing the intent?

@bernaferrari
Copy link
Owner

bernaferrari commented Jul 4, 2024 via email

@davestewart
Copy link
Contributor

davestewart commented Jul 16, 2024

Hey @bernaferrari.

This is probably just me not understanding optimize layout, but can you help me understand something?

I thought for a while I may have discovered a possible issue with commonSortChildrenWhenInferredAutoLayout() when layoutWrap is set to "WRAP" – but I might be mistaken.

Because the algorithm sorts on x position when layoutMode is "HORIZONTAL" wrapping elements will be sorted out of order.

Optimise layout is off, layers render in order (see code):

image

Optimise layout is on, layers render out of order (see code) because they are sorted in x:

image

However, the container pictured above already has Figma auto-layout on (set to HORIZONTAL) so should it not just return the existing children in order?

https://github.com/bernaferrari/FigmaToCode/blob/main/packages/backend/src/common/commonChildrenOrder.ts#L8

  // add this line
  if ('layoutMode' in node && node.layoutMode && node.layoutMode !== 'NONE') {
    return node.children
  }

I appreciate it might have been a while since you looked at this code, but can you confirm?

Thanks!

@bernaferrari
Copy link
Owner

inferred doesn't return the childrens ordered, so I try to guess their order :P

@davestewart
Copy link
Contributor

Sure, but when an object already has auto-layout, ordering does not need to be guessed.

My point is, this function is being called from <platform>Frame() even when auto-layout is on (and I don't think it should be).

CleanShot 2024-07-17 at 00 40 36

Am I wrong?

@bernaferrari
Copy link
Owner

oh, interesting. Yeah, you found a bug!!

@davestewart
Copy link
Contributor

OK! Thanks for confirming. I'll fix it along with the wrapping implementation.

@davestewart
Copy link
Contributor

davestewart commented Jul 17, 2024

So I finally figured out what inferred auto layout is 😅

Any layout for which you could click "Auto Layout" and the layout would be rendered exactly the same once auto-layout properties such as gap, padding and alignment were applied.

CleanShot 2024-07-17 at 01 07 48

green: layout would be the same after auto-layout applied, red: layout would not be the same

Not sure why they don't say this in the docs!

@bernaferrari
Copy link
Owner

bernaferrari commented Jul 17, 2024 via email

@davestewart
Copy link
Contributor

No worries. Thanks for helping get up to speed on the Figma API and answering all my questions!

@davestewart
Copy link
Contributor

davestewart commented Jul 17, 2024

Interestingly, inferredAutoLayout is null for potentially wrapped elements, even when clicking the auto-layout button correctly applies wrap to the layout:

CleanShot 2024-07-17 at 01 21 18

Suspect the heuristics only take into account the general offsets and padding, etc.

@davestewart
Copy link
Contributor

davestewart commented Jul 17, 2024

FWIW, some of the UI options could be disabled if they are not relevant:

  • "Optimize Layout" – if there are no inferredAutoLayout nodes
  • "Round Values" and "Round Colors" – if no [] or / characters found in classes
  • "Custom Colors" – if no color variable used

@bernaferrari
Copy link
Owner

if I enable inferredAutoLayout, click in a layout without, then click in a layout with it, will the option work?

If it is documented somewhere that they are disabled because there is nothing, I guess it is fine. Or could have some sort of section like "unused on current selection:"

@davestewart
Copy link
Contributor

davestewart commented Jul 17, 2024

Ah, I see what you mean.

I suppose I was thinking about making it clearer for users who might click the options (when irrelevant) and wonder why they were not having any effect.

Not a big deal, just a nice-to-have. Can save it for later!

@bernaferrari
Copy link
Owner

we can have that disabled, as long as it is clear for users (even if irrelevant). Another way is add on the tooltip: "current selection doesn't make use of color variables". with a check or X. Could be useful for people.

@davestewart
Copy link
Contributor

Will add a feature request issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants