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

"data-flex" doesnt allow for granular control over 'grow' and 'shrink' properties #34

Open
drew0530 opened this issue Aug 20, 2024 · 3 comments

Comments

@drew0530
Copy link

drew0530 commented Aug 20, 2024

  • I was wrong here, Flex Layout uses flex: 1 1 auto as default, but keep reading for the grow + shrink stuff.

Issue

The default behavior for using data-flex should be a direct reflection of its CSS counterpart, the flex property. Default behavior is flex: 0 1 auto. Currently, this library has data-flex defaulted to flex: 1 1 auto (see Flex - MDN). You can override the grow property, but then you cannot define the flex-basis, it is set to auto unless it is overridden through another class or with a style attribute declaration.

Solution

While I think that ultimately setting 'flex 0 1 {%}' for data-flex would be the most straightforward answer to this issue, I understand that this library is being used by quite a few people to assist with their migration process, so changing the default behavior of data-flex will likely ruffle some feathers. Because of this, I have a few alternate ideas on how to solve this.

Currently, there is no way to include a flex-basis while also defining the grow/shrink properties. You can only use data-flex="nogrow" (which equates to flex: 0 1 auto) OR data-flex="{percentage}". Using both will just override the first attribute. Again, we have to say style="flex-grow: 0" in conjunction with data-flex="50" in order to produce default behavior.

Examples

a. <div data-flex-intial="50%"></div>
b. <div data-flex="noshrink 50%"></div>
c. <div data-flex="50" data-flex-nogrow>

I can provide a pull request for these changes, but I wanted to get a sense of direction before I put any real work into applying a solution.

@philmtd
Copy link
Owner

philmtd commented Sep 1, 2024

Hi @drew0530, thanks for submitting an issue - and excuse my late reaction due to the holiday season.

Not trying to be nitpicky but I do not think the title of the issue is correct. This library is not causing display issues but is rather not behaving the way you think it should. And I don't think that your ultimate solution would be the most straightforward answer to the issue, either: This library is behaving exactly the way it is intended to - inspired by Angular Flex-Layout, just in pure CSS. That is kind of the "API" that is promised here and I do not intend to break it.

I think of your suggested examples I'd prefer b. so either data-flex="nogrow 50%" or (maybe easier to match in CSS) data-flex="50% nogrow" etc. I think this should be doable similar to other attributes that match more than one term in the attribute value. I am just not sure how to incorporate it with the CSS classes, but am also not sure if it is necessary (even though I initially wanted to keep both ways similar regarding the features).

If you wish to work on a solution (similar to b.), you're welcome to submit a PR. I would just need you to not change any current default behaviour.

@drew0530
Copy link
Author

drew0530 commented Sep 3, 2024

No worries, and I hope the holiday was well for you.

I will work on a pull request as I can find the time to, since my team and I are in the middle of migrating away from flexLayout right now. I'd also like to add that I was wrong in my assumption that flexLayout was using flex: 0 1 as it's defaults, instead of using the default css values, so I was looking in the wrong places to explain why my templates weren't migrating cleanly. Sorry for assuming this project was just randomly setting the grow property to 1!

I will go ahead and change the title of this issue since it doesn't really reflect the fix... which I would actually like to offer another example for; which is from the fxFlex API. I specifically like their 'long-form' vs 'short-form' options, which would look like this:

Short-form (typical usage)

<div data-flex="<basis>"></div>

Long-form (typical usage)

<div data-flex="<grow> <shrink> <basis>"></div>

This way offers the user more granular control over their grow and shrink properties and ultimately should provide a more clean solution for users migrating from this syntax. Let me know what you think and I can get started.

@drew0530 drew0530 changed the title Defaulting data-flex to "flex: 1 1 auto" causing display issues "data-flex" doesnt allow for granular control over 'grow' and 'shrink' properties Sep 3, 2024
@philmtd
Copy link
Owner

philmtd commented Sep 10, 2024

Sorry, I did not get an email for your edit and only just checked your comment here to see it is much longer now 😅

Thanks. And yes sure, go ahead. I think having a grow-shrink-basis attribute would be a nice addition if you can get it to work alongside the short form. I think I never tried to implement it, but working with the limited abilities of CSS selectors could be a challenge.

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

2 participants