Skip to content

Conversation

@cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jun 30, 2022

Summary

Before

After

Repro steps:

This bug only occurs on sorted header cells with actions disabled (i.e. no popover wrapper).

  1. Go to https://eui.elastic.co/v60.1.0/#/tabular-content/data-grid
  2. Click the "Sort fields" toolbar button
  3. Sort by the "Version" field (any direction)
  4. Notice the broken header cell display with only the arrow showing

Cause

This bug appears to have been caused by the Emotion conversion of EuiIcon. Because Emotion CSS comes before our remaining Sass CSS, the width of the icon is being overridden by the > * { width: 100%; } CSS. This will in theory(?) fix itself once we're fully on Emotion, but since that's a whiles away, this fix solves the issue in the interim by removing the > * blanket selector and only applying our .eui-fullWidth CSS utilities to specific divs that we need it.

As far as I can tell the removal of max-width: 100%; has no impact - .euiPopover already has max-width 100% and .euiDataGridHeaderCell__content also has a max-width of 100% due to its inheritance of our text truncation CSS.

Checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • A changelog entry exists and is marked appropriately

- [ ] Props have proper autodocs and playground toggles
- [ ] Added documentation
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Added or updated jest and cypress tests
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Checked for accessibility including keyboard-only and screenreader modes
- [ ] Updated the Figma library counterpart

@cee-chen cee-chen requested a review from chandlerprall June 30, 2022 16:36
@cee-chen cee-chen marked this pull request as ready for review June 30, 2022 16:41
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6014/

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

LGTM! Header titles no longer disappear and the popover display & interactions continue functioning as expected.

@cee-chen
Copy link
Contributor Author

Thanks Chandler! I'll have a couple more column header PRs coming your way as a heads up - primarily around accessibility improvements. 💪

@cee-chen cee-chen enabled auto-merge (squash) June 30, 2022 17:25
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6014/

@cee-chen cee-chen merged commit b63fbce into elastic:main Jun 30, 2022
@cee-chen cee-chen deleted the datagrid/column-header-bug branch June 30, 2022 18:01
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