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.

Sign up for GitHub

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

Jump to bottom

[Emotion] Convert EuiDescriptionList #5971

Merged
merged 66 commits into from
Jul 27, 2022
Merged

[Emotion] Convert EuiDescriptionList #5971

merged 66 commits into from
Jul 27, 2022

Conversation

breehall
Copy link
Contributor

@breehall breehall commented Jun 15, 2022 •
edited
Loading

Summary

This PR converts EuiDescriptionList and the euiBreakpoint mixin to Emotion

Updated the EuiDescriptionListTitle and EuiDescriptionListDescription by allowing them to contain their own styles based on the align, type, textStyle, and compressed props. These components receive access to the props from the new EuiDescriptionListContext which passes down the four props.

While this is a change in architecture, I don't believe that this would be a breaking change because nothing will change with the implementation for EuiDescriptionList.

Updated EuiDescriptionList docs examples to give more insight on using EuiDescriptionListTitle and EuiDescriptionListDescription manually.

Checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • 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
  • A changelog entry exists and is marked appropriately

@cee-chen
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

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

@breehall
Resolved bug in descriptionlist column styling that caused list eleme…
94cca23 
…nts to be displayed with flex instead of block
@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@breehall breehall marked this pull request as ready for review June 21, 2022 00:27
@breehall breehall mentioned this pull request Jun 22, 2022
Closed
49 tasks
@breehall breehall requested a review from cee-chen June 27, 2022 13:42
@breehall
Copy link
Contributor Author

Hey @constancecchen, do you mind reviewing this?

@cee-chen
Copy link
Contributor

Hey Bree! No problem, but as a heads up I'm currently working on the latest Kibana EUI upgrade and reviewing Elizabet's EuiIcon PR, so it may take me a day to get to reviewing this PR.

@breehall
Copy link
Contributor Author

@constancecchen It completely slipped my mind that you're currently working on the upgrade! I don't want to overload you. Is it ok if I reassign it?

@cchaos
Copy link
Contributor

cchaos commented Jun 27, 2022

@breehall I can take a pass at it for you today

@breehall
Copy link
Contributor Author

@cchaos Awesome, thank you!

@kibanamachine
Copy link

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

@breehall breehall marked this pull request as draft June 27, 2022 19:41
breehall and others added 11 commits July 26, 2022 10:10
@breehall
Update src/components/description_list/description_list_title.styles.ts
628c0f5 
Co-authored-by: Constance <[email protected]>
@breehall
Update src/components/description_list/description_list_title.styles.ts
6a46bb2 
Co-authored-by: Constance <[email protected]>
@breehall
Update src/components/description_list/description_list_title.styles.ts
c88604c 
Co-authored-by: Constance <[email protected]>
@breehall
Update src/components/description_list/description_list_description.s…
4de25a2 
…tyles.ts

Co-authored-by: Constance <[email protected]>
@breehall
Update src/components/description_list/description_list_title.styles.ts
e4e0b58 
Co-authored-by: Constance <[email protected]>
@breehall
Update src/components/description_list/description_list_title.styles.ts
773fe6a 
Co-authored-by: Constance <[email protected]>
@breehall
Update src/components/description_list/description_list_title.styles.ts
5e88e0d 
Co-authored-by: Constance <[email protected]>
@breehall
Update src/components/description_list/description_list_title.styles.ts
c296617 
Co-authored-by: Constance <[email protected]>
@breehall
Removed classNames for the type, align, textStyle, and compressed pro…
dd57a4b 
…ps. The type prop will be available by accessing the data-type attribute on EuiDescriptionList
@breehall
Merge branch 'main' of https://github.com/breehall/eui into emotion/d…
dfe232e 
…escription-list

Pulling in main branch
@breehall
Merge branch 'emotion/description-list' of https://github.com/breehal…
d82ebf2 
…l/eui into emotion/description-list

Pulling in current code from emotion/description-list branch
@kibanamachine
Copy link

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

@breehall
Updated parameters of euiBreakpoint to match the latest iteration of …
f1f73d6 
…the function. Updated snapshots that were required from the move of the data-type attribute from the child DescriptionList elements to the parent
@kibanamachine
Copy link

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

@cee-chen
Copy link
Contributor

@breehall Looking good! My only blocking comments are on on the xl media query selector, everything else is optional.

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

cee-chen
cee-chen approved these changes Jul 27, 2022
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

🎉 It's happening!! Thanks for your patience with all my feedback!

@kibanamachine
Copy link

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

@breehall breehall merged commit 59df738 into elastic:main Jul 27, 2022
@thompsongl thompsongl mentioned this pull request Aug 15, 2022
Merged
@cee-chen cee-chen mentioned this pull request Sep 13, 2023
Merged
4 tasks
@breehall breehall deleted the emotion/description-list branch September 18, 2023 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TGspace TGspace TGspace left review comments

@thompsongl thompsongl thompsongl left review comments

@cchaos cchaos cchaos left review comments

@cee-chen cee-chen cee-chen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@breehall @cee-chen @kibanamachine @cchaos @thompsongl @TGspace