Skip to content

Add a responsiveHeader prop to the EuiAccordion#2141

Closed
sulemanof wants to merge 1 commit intoelastic:masterfrom
sulemanof:add_responsive
Closed

Add a responsiveHeader prop to the EuiAccordion#2141
sulemanof wants to merge 1 commit intoelastic:masterfrom
sulemanof:add_responsive

Conversation

@sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Jul 19, 2019

Summary

While using the accordion on small screens there could be a necessity not to wrap buttons on the right side:

eui_responsive

Such a necessity was caused in kibana default vis editor (in the metric euiFlexGroup--responsive is deleted to show the desired behavior):

kibana_responsive

Checklist

  • This was checked in mobile
  • This was checked in IE11
  • This was checked in dark mode
  • Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
  • This was checked against keyboard-only and screenreader scenarios
  • This required updates to Framer X components

<EuiFlexGroup
gutterSize="none"
alignItems="center"
responsive={responsiveHeader}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sulemanof ! I actually think this just has been overlooked since there are few instances of accordions using the extra actions. However, I don't think there's a reason responsive should ever be true. The inner label will just wrap and everything else will just stay center aligned. How about we remove this new prop and just change this to be

Suggested change
responsive={responsiveHeader}>
responsive={false}>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, looking at @maryia-lapata's PR and the nested craziness of selectors in this file https://github.com/elastic/kibana/pull/40866/files#diff-1235e98f96e13b1964ee5a0e1b0ff94d makes me think we need to remove the EuiFlexGroup dependency all together from EuiAccordion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's largely due to the negative margin issue with our flex system. It causes really dumb problems with anything that has an overflow applied (like accordion). #937

We could either address it there or simply make something more specific in accordion like you mention. Likely though, people would just add flex groups back into it for more complicated layouts.

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

Successfully merging this pull request may close these issues.

3 participants