-
Notifications
You must be signed in to change notification settings - Fork 841
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
Add SuggestItem #2061
Comments
Great writeup. I think your breakdown looks good, just had a couple suggestions.
This word can be confusing in the way we talk about EUI because "user" to us usually means the end -user of Kibana. However, when what we really mean is someone who is writing code using EUI we tend to refer to as the consumer. It just helps us distinguish between what we might be exposing as a setting to the user versus a prop for the developer. That said, I agree that allowing the consumer to choose their icon/color combinations is a good idea. However, instead of splitting it up between
Then The prop As for using I'd suggest just using your own wrapping divs, spans, whatever, and using custom classes and CSS to create your layout. You'll most likely still use the flexbox model, but it gives you the best flexibility. We try to shy away from custom classes and CSS where possible in consuming apps like Kibana, mainly because it's hard to maintain over there. But here in EUI, it's perfectly good to do so and we even promote it because then you won't have inheritance issues if there's ever a bug in EuiFlexGroup. |
Looks great! +1 regarding not letting the consumer supply a background color; calculating it seems preferable. There are some structural similarities to other existing components... might be worth poking around in those files: thinking of I suspect you may end up needing an It sounds like you're building this out in pieces and my only other thoughts in looking at this were how to handle things like the number of rows being displayed before scrolling starts (ie. popover height) and if we should let consumers set a width on the |
@cchaos thanks for the input, super useful.
In the case where the consumer passes in a hex value for |
That's a very good question. I think there are two paths you can follow:
|
Implemented in #2090 |
As discussed with Dave, the following would be the approach to replace the interface shown above with Eui components. For reference, that interface can be found at the top of Dashboard and Discover in Kibana.
Kibana currently builds this interface using some existing Eui components (namely
EuiIcon
). Kibana passes thetype
prop toEuiIcon
. It also creates a CSS class{kbnSuggestionItem-- + props.suggestion.type}
and calculates color and background-color in SASS for eachtype
(this is for the square which contains the icon on each suggestion). See:In the current implementation each
type
is linked to a specific color and background-color.I would like for
EuiSuggestItem
to offer the user more freedom and allow them pick the color for the icon they're using. For that reason,EuiSuggestItem
'stype
prop would be limited to generating the icon throughEuiIcon
only. I would add other props to handle color.Here are the props that I’m considering:
type
=> to generate the icon usingEuiIcon
(Kibana currently usesEuiIcon
for this and I would like to do the same)text
=> primary text / labeldescription
=> secondary text (optional)color
=> color of the icon. Accepts either our palette colors (primary, secondary ..etc) or a hex value.bgcolor
=> background color of the square where the icon is placed. Accepts hex value.In the case in which the user picks a palette color for
color
we would auto-generate the background-color for them in SASS. Unless they specify a hex value forbgcolor
.In terms for structure I was thinking of something like this:
Please send your comments, suggestions, etc.
The text was updated successfully, but these errors were encountered: