-
Notifications
You must be signed in to change notification settings - Fork 152
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(Stack): do not apply flex сlasses, when not needed #4162
Conversation
flex || inline ? getDisplayClasses(inline ? "inline-flex" : "flex", viewport) : "block", | ||
getSpacingClasses(spacing, viewport, direction, legacy), | ||
inline === false && "w-full", | ||
inline === false && direction === DIRECTION.COLUMN && "h-full", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a prop called justify that is justify-content in flex-box, it works as expected with direction='row' (in case it’s not inlined with inline prop), but not with direction='column'
for instance, I expected it to be at the end with justify='end', but it’s placed in the beginning, the same goes for other values: around, between, and so on. To work as expected, the Stack div container should have a height: 100%, the same as with the row case, when it has a width: 100% (I think we should look at it as x and y axes and it should have 100% for justify prop)
I think we are missing it 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you suggest that we automatically apply width=100%
when direction=row
and height=100%
when direction=column
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the width is already applied, when it has no inline, which makes sense, the same is for height, we may add additional condition to apply it only when justify is provided 🤔
Size Change: +34 B (0%) Total Size: 460 kB
ℹ️ View Unchanged
|
9f6ce15
to
9059903
Compare
9059903
to
73fd914
Compare
73fd914
to
07f94d1
Compare
Giving solution to FEPLT-1833
While working on fixing the flex styles issues, I also found an interesting behavior, I'd say unexpected, described below in the PR.
Storybook: https://orbit-mainframev-fix-stack-issue.surge.sh