-
Notifications
You must be signed in to change notification settings - Fork 826
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
[Emotion] Convert EuiDescriptionList #5971
Conversation
…escription-list Merge in latest code from main
…EuiDescriptionList styles
…ted the euiBreakopint mixin to TypeScript
…escription-list Pulling in code from main
jenkins test this |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
…nts to be displayed with flex instead of block
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
…escription-list Pulling in the latest code from main
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
src/components/description_list/__snapshots__/description_list.test.tsx.snap
Outdated
Show resolved
Hide resolved
Hey @constancecchen, do you mind reviewing this? |
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. |
@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? |
@breehall I can take a pass at it for you today |
@cchaos Awesome, thank you! |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
…tyles.ts Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
…ps. The type prop will be available by accessing the data-type attribute on EuiDescriptionList
…escription-list Pulling in main branch
…l/eui into emotion/description-list Pulling in current code from emotion/description-list branch
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
…the function. Updated snapshots that were required from the move of the data-type attribute from the child DescriptionList elements to the parent
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
src/components/description_list/description_list_description.styles.ts
Outdated
Show resolved
Hide resolved
src/components/description_list/description_list_title.styles.ts
Outdated
Show resolved
Hide resolved
src/components/description_list/description_list_title.styles.ts
Outdated
Show resolved
Hide resolved
@breehall Looking good! My only blocking comments are on on the |
Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
There was a problem hiding this 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!
Preview documentation changes for this PR: https://eui.elastic.co/pr_5971/ |
Summary
This PR converts
EuiDescriptionList
and theeuiBreakpoint
mixin to EmotionUpdated the
EuiDescriptionListTitle
andEuiDescriptionListDescription
by allowing them to contain their own styles based on thealign
,type
,textStyle
, andcompressed
props. These components receive access to the props from the newEuiDescriptionListContext
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 usingEuiDescriptionListTitle
andEuiDescriptionListDescription
manually.Checklist
Props have proper autodocs and playground togglesUpdated the Figma library counterpart