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

[EuiFlexGroup] When direction="column" we should set responsive={false} or adjust the styling to ignore responsive css #5529

Closed
snide opened this issue Jan 11, 2022 · 3 comments · Fixed by #6270

Comments

@snide
Copy link
Contributor

snide commented Jan 11, 2022

When diagnosing elastic/kibana#121403 with @gavinwye we saw lots of EuiFlexGroup components in Security were set to direction="column". Along with the default responsive={true} this can cause items to grow inside containers in weird ways because of the flex-basis moving to 100% for children.

I can't think of an instance where we'd want to apply the flex-basis on the column layout to 100% in mobile. Likely the easiest solution is to make the CSS for the responsive selector to more specific so it doesn't do anything when in a column layout. Another option would be to make responsive={false} in the component itself whenever direction="column". I think I prefer handling this at the CSS level.

https://github.com/elastic/eui/blob/main/src/components/flex/_flex_item.scss#L31-L41

@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.

@cee-chen
Copy link
Contributor

This still seems valid to resolve.

@cee-chen cee-chen reopened this Jul 20, 2022
cee-chen added a commit to cee-chen/eui that referenced this issue Sep 27, 2022
…column directions

- Due to switch to `gap`, no need for margin overrides anymore - the gap will be calculated automatically

- Due to flatter Emotion classes, no need for !important anymore (which is probably extremely annoying to override)

- see elastic#5529 for column directions decision
cee-chen pushed a commit that referenced this issue Sep 30, 2022
* Convert EuiFlexGroup to Emotion

+ convert gutter CSS to `gap`
+ remove className maps
+ clean up unnecessary `...rest` cast

+ convert nested EuiFlexItem to flattened styles within flex item styles - realistically, who's going to be using EuiFlexItem outside of EuiFlexGroup?

* Convert EuiFlexItem responsive CSS and remove responsive styling for column directions

- Due to switch to `gap`, no need for margin overrides anymore - the gap will be calculated automatically

- Due to flatter Emotion classes, no need for !important anymore (which is probably extremely annoying to override)

- see #5529 for column directions decision

* Convert EuiFlexItem to Emotion + support grow={0}

- separate out `grow` logic via JS to reduce CSS overrides
- explicitly print grow-1 class if grow is a generic `true`

+ improve props table
+ remove `@ts-ignore` by switching type to ElementType
+ add missing `component` prop unit test

* Update all downstream snapshots

* Changelog

* [EuiFlexGrid] Remove `columns={0}` as an option

+ improve props docs

* [Docs] convert remaining .js examples to .tsx

* [PR feedback][docs] Remove negative margin callout

* changelog for #4140 bugfix

* [PR feedback] Add responsive flex grid example

+ move responsive flex group example up above grid examples

* [Docs] Misc code cleanup

- combine src/components imports
- remove unnecessary div wrappers

* [Docs] Remove _flex.scss in favor of an Emotion wrapper

* [Docs] move responsive flex grid example to better match flow of page

* Fix playgrounds to accept custom Emotion CSS / fix flex playgrounds

- I probably should not have tried to tackle removing doc example Sass in this PR, sorry Elizabet 🙈
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.

3 participants