Skip to content

[EuiDataGrid] Add lineHeight configuration to rowHeightsOptions#5221

Merged
cee-chen merged 8 commits intoelastic:masterfrom
cee-chen:datagrid-lineheight
Sep 30, 2021
Merged

[EuiDataGrid] Add lineHeight configuration to rowHeightsOptions#5221
cee-chen merged 8 commits intoelastic:masterfrom
cee-chen:datagrid-lineheight

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen commented Sep 28, 2021

Summary

Original concept in #5140 and heavy lifting done by @chandlerprall, I'm just carrying this over the finish line in terms of opening up a PR.

Documentation example with lineHeight: '2em':

Example with lineHeight: '1' (unitless):

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile

  • Checked in Chrome, Safari, Edge, and Firefox

- [ ] Props have proper autodocs and playground toggles

- [ ] Checked Code Sandbox works for any docs examples
- [ ] Added or updated jest tests (Skipped because I've yet to write Jest tests for row_height_utils)
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Checked for accessibility including keyboard-only and screenreader modes

  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link
Copy Markdown

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

chandlerprall and others added 4 commits September 28, 2021 14:14
- Add new example that uses same dataset as other 2 on page, but makes it text only (lineHeight doesn't work as expected with nested <EuiText>

- Update existing documentation, make 'fixed' example more of a demo of rowHeights overrides
- Fix awkward grammar which was tweaked in another spot in the docs

- fix defaultHeight snippet example that was missing a closing brace (change it to static #, since I added a lineCount example already earlier in the page)

- fix redundant =
@cee-chen cee-chen marked this pull request as ready for review September 28, 2021 21:15
@cee-chen
Copy link
Copy Markdown
Contributor Author

FYI @ryankeairns and @andreadelrio, thanks for your patience! This should hopefully be available next release

@cee-chen
Copy link
Copy Markdown
Contributor Author

jenkins test this

@kibanamachine
Copy link
Copy Markdown

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

@elizabetdev elizabetdev self-requested a review September 29, 2021 16:03
@cee-chen
Copy link
Copy Markdown
Contributor Author

jenkins test this

@kibanamachine
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

Thanks, @constancecchen! LGTM! 🎉

Tested in Chrome, Safari, Edge, and Firefox. Also did some tests locally.

Just added small suggestions to the docs.

I also noticed that on the following page the rowHeightsOptions prop is missing in both snippet and general props: https://eui.elastic.co/pr_5221/#/tabular-content/data-grid#snippet-with-every-feature-in-use.

Constance and others added 3 commits September 30, 2021 09:00
@kibanamachine
Copy link
Copy Markdown

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

@cee-chen
Copy link
Copy Markdown
Contributor Author

55a1069 should address

I also noticed that on the following page the rowHeightsOptions prop is missing in both snippet and general props: https://eui.elastic.co/pr_5221/#/tabular-content/data-grid#snippet-with-every-feature-in-use.

@cee-chen cee-chen enabled auto-merge (squash) September 30, 2021 16:26
@kibanamachine
Copy link
Copy Markdown

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

@cee-chen cee-chen merged commit 9a5b78d into elastic:master Sep 30, 2021
@cee-chen cee-chen deleted the datagrid-lineheight branch September 30, 2021 17:02
@ryankeairns
Copy link
Copy Markdown
Contributor

Thanks @constancecchen !

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.

5 participants