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.

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

[New Nav Feature] Added ghost colored EuiListGroupItem #3018

Merged

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Mar 9, 2020

Adds one more option to EuiListGroupItem for color

which is ghost, for use when the list is on a dark background as will be needed like in this screenshot:

Screen Shot 2020-03-09 at 16 11 36 PM

Also, adds the option for EuiListGroup to pass the color option to every list item

Typically you'd only be able to color each list item, but now you color all of them if you're passing items via the listItems prop object.

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • [ ] Checked in mobile
  • Checked in IE11 and Firefox
  • Props have proper autodocs
  • Added documentation examples
  • Added or updated jest tests
  • [ ] Checked for breaking changes and labeled appropriately
  • [ ] Checked for accessibility including keyboard-only and screenreader modes
  • [ ] A changelog entry exists and is marked appropriately Will get added via feature branch

Comment on lines +32 to +35
.euiAccordion__iconWrapper {
margin-left: $euiSizeS;
margin-right: $euiSizeXS;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also includes a quick fix to the PR that included the right-side arrow accordion

cchaos added 4 commits March 9, 2020 16:28
@cchaos cchaos force-pushed the props/listgroupitem_ghost branch from bf9ed47 to bc94df6 Compare March 9, 2020 20:28
@kibanamachine
Copy link

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

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Code looks OK. Looks like the subdued example has an issue with focus state

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

Code changes LGTM

@cchaos
Copy link
Contributor Author

cchaos commented Mar 9, 2020

Looks like the subdued example has an issue with focus state

Good catch and upon further investigation it looks like it's an issue with the large sizing, not subdued. 👀

Copy link
Contributor

@andreadelrio andreadelrio left a comment

Choose a reason for hiding this comment

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

Changes LGTM but had a question about the ghost example you added to the docs.

src-docs/src/views/list_group/list_group_item_color.tsx Outdated Show resolved Hide resolved
@cchaos cchaos requested a review from snide March 9, 2020 21:23
@cchaos
Copy link
Contributor Author

cchaos commented Mar 9, 2020

The issue with the underline on the large sized items was that it was getting cutoff by such a short line-height. I thought it was a bit cramped to try and keep the same height as teh medium and small sizes, so I just increased the overall heigh of large items by increasing the line-height to 40px from 32px.

@kibanamachine
Copy link

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

@cchaos cchaos merged commit 09b51a5 into elastic:feature/collapsible_nav Mar 9, 2020
@cchaos cchaos deleted the props/listgroupitem_ghost branch March 9, 2020 22:46
@cchaos cchaos mentioned this pull request Mar 9, 2020
8 tasks
cchaos added a commit that referenced this pull request Mar 17, 2020
* Added color=ghost to list group item

And fixed class names for list group

* Added `color` prop to EuiListGroup

Fixed color on disabled list group items

* Fixing hover colors for each list item color

* ghost example

* Increase the height of large items too

Fixes the hidden underline in focus state

* Fixing demo to not apply black bg to list item

* Snaps
cchaos added a commit that referenced this pull request Mar 18, 2020
* Added color=ghost to list group item

And fixed class names for list group

* Added `color` prop to EuiListGroup

Fixed color on disabled list group items

* Fixing hover colors for each list item color

* ghost example

* Increase the height of large items too

Fixes the hidden underline in focus state

* Fixing demo to not apply black bg to list item

* Snaps
cchaos added a commit that referenced this pull request Mar 26, 2020
* Added color=ghost to list group item

And fixed class names for list group

* Added `color` prop to EuiListGroup

Fixed color on disabled list group items

* Fixing hover colors for each list item color

* ghost example

* Increase the height of large items too

Fixes the hidden underline in focus state

* Fixing demo to not apply black bg to list item

* Snaps
cchaos added a commit that referenced this pull request Mar 27, 2020
* [Feature] Added `EuiCollapsibleNav` component (#2977)
* [New Nav Feature] Added `ghost` colored EuiListGroupItem (#3018)
* [New Nav Feature] Created `EuiCollapsibleGroup` (#3031)
* [New Nav Feature] EuiPinnableListGroup (#3061)
* [K8 Nav Feature] Added `home` and `menu` glyphs to EuiIcon (#3109)
* [New Nav Feature] Final docs examples and patterns (#3117)
* [New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav (#3168)
miukimiu pushed a commit to miukimiu/eui that referenced this pull request Mar 30, 2020
* [Feature] Added `EuiCollapsibleNav` component (elastic#2977)
* [New Nav Feature] Added `ghost` colored EuiListGroupItem (elastic#3018)
* [New Nav Feature] Created `EuiCollapsibleGroup` (elastic#3031)
* [New Nav Feature] EuiPinnableListGroup (elastic#3061)
* [K8 Nav Feature] Added `home` and `menu` glyphs to EuiIcon (elastic#3109)
* [New Nav Feature] Final docs examples and patterns (elastic#3117)
* [New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav (elastic#3168)
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