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

[EuiFlexGrid] Add justifyContent and alignItems like EuiFlexGroup #4878

Closed
j-m opened this issue Jun 14, 2021 · 10 comments · Fixed by #6281
Closed

[EuiFlexGrid] Add justifyContent and alignItems like EuiFlexGroup #4878

j-m opened this issue Jun 14, 2021 · 10 comments · Fixed by #6281

Comments

@j-m
Copy link
Contributor

j-m commented Jun 14, 2021

EuiFlexGroup has justifyContent and alignItems props, but EuiFlexGrid surprisingly doesn't.

@j-m
Copy link
Contributor Author

j-m commented Jun 16, 2021

@cchaos Seeing as EuiFlexItem is also display: flex, should EuiFlexItem also have these props?

@cchaos
Copy link
Contributor

cchaos commented Jun 16, 2021

We've stayed away from adding those because it's a lot to maintain and then item is no longer different from group. The flex is mainly added to allow for stretching. We've instead instructed people to next EuiFlexGroup inside if they need more flex adjustments, or wrap their children <div> to get out of the flex model.

@bhargavjoshi
Copy link

I think we should close this issue.

@cchaos
Copy link
Contributor

cchaos commented Jun 21, 2021

The issue is still valid for EuiFlexGrid, just not EuiFlexItem.

@hetanthakkar
Copy link
Contributor

I am on it!

@hetanthakkar
Copy link
Contributor

@cchaos @j-m By adding justify-content and align-items props, we also add the corresponding styling for the component when that prop is passed right!?

@j-m
Copy link
Contributor Author

j-m commented Jun 23, 2021

@cchaos FlexGroup and FlexGrid are incredibly similar, AFAICT grid realistically should just be a group with an implicit wrap and extra columns prop.

Grid is also missing reverse directions, as well a check on the component prop (group's is limited to div/span).

A breaking change to unify them and deprecate grid is probably a little OTT, but maybe grid should just use group under the hood?

@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@github-actions
Copy link

❌ We're automatically closing this issue due to lack of activity. Please comment if you feel this was done in error.

@cchaos
Copy link
Contributor

cchaos commented Jan 6, 2022

Re-opening. Still low-hanging fruit. Most likely to get tackled during the CSS-in-JS conversion.

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

Successfully merging a pull request may close this issue.

4 participants