oui

oui copied to clipboard

Description

Props shrink and basis have been added for OuiFlexItem. Behaviour similar to grow prop. Accepted values:

• shrink: true, false, null, 1..10
• basis: true, false, null, 'auto', 'max-content', 'min-content', 'fit-content',

Issues Resolved

Fixes issue #776

Check List

• [x] New functionality includes testing.
• [x] New functionality has been documented.
• [x] All tests pass
  • [x] yarn lint
  • [x] yarn test-unit
• [x] Update CHANGELOG.md
• [x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@BSFishy @joshuarrrr,

Final updates in PR are ready, with documentation added for props shrink and basis. I would appreciate your feedback on the entire PR.

Just one thing from my side. I think it is a bit cumbersome to consider OuiFlexItem to have the properties flex-grow: 1 and flex-basis: 0%, by default. I feel that we should follow an approach like the one here in W3School, where we can explain the props grow, shrink and basis altogether in one section and also not considering a default behaviour of growing.

Not sure. Maybe the UX team have something to say here.

Regards,

Samuel

Hi @BSFishy,

I already replied to your comments for your code review. Looking forward your reply to see how I proceed.

Regards,

Samuel

@BSFishy,

Changes added as requested. My only issue would be where we should describe the default behaviours for grow, shrink and basis in the documentation.

So for grow and shrink props we will have:

• if null/undefined/false/0: deactivate default behaviour for flex-grow and/or flex-shrink of ouiFlexItem -> apply class ouiFlexItem--flexGrowZero and/or ouiFlexItem--flexShrinkZero which are equivalent to flex-shrink:0 and flex-basis:auto and/or flex-shrink:0 and flex-basis:auto, respectivetely.
• if true: use default component behaviour -> flex-grow: 1 and/or flex-shrink:1 and flex-basis:0% -> inherited from styles defined in _flex_group.scss for class .ouiFlexItem (previous from my work ion this issue)
• if 1-9: grow and/or shrink the component using a ratio, keeping flex-basis: 0%. For example, if two flex items with grow = 2 and grow = 3, respectively, the expected growing ratio would be proportional to those values. Sames apply when shrinking.

For basis prop we will have:

• if null/undefined/false/0: deactivate default behaviour for flex-basis-> apply class ouiFlexItem--flexBasisAuto equivalent to flex-basis:auto.
• if true: use default component behaviour -> flex-basis: 0% -> inherited from styles defined in _flex_group.scss for class .ouiFlexItem (previous from my work ion this issue)
• if auto, max-content, min-content or fit-content: apply one of those properties that will equate to flex-basis: auto, flex-basis: max-content, flex-basis: min-content or flex-basis: fit-content.

Hope this explanation above is clear.

Regards,

Samuel

@BSFishy,

Updates are ready with the last code review done from your side! Any comments, let me know.

Regards,

Samuel

Looks great!

Hi @BSFishy, @joshuarrrr

Are we good to go with this PR? The branch was recently updated with main bracnh.

Are we good to go with this PR? The branch was recently updated with main bracnh.

Just need another approval

@AMoo-Miki, @kolchfa-aws,

Sorry bothering again with this PR, but I would appreciate your feedback on the reviews Mikki sent to see what other changes you need from my side.

Regards,

Samuel

@BigSamu I made suggestions where Miki flagged the doc team. Let me know if you need any other help from me.

@BigSamu, I made suggestions where Miki flagged the doc team. Let me know if you need any other help from me.

Thanks @kolchfa-aws,

I will review this week.

@AMoo-Miki,

Any feedback from the suggestions from @kolchfa-aws and also the questions I post on your comments?

Regards,

Samuel

@AMoo-Miki,

Have you had the chance to review the comments from @kolchfa-aws? I would appreciate your feedback for moving forward on this PR.

Regards,

Samuel