components: extend Aggregation and EmptyResults interface #128
Conversation
src/lib/components/BucketAggregation/BucketAggregationValues.js
Outdated
Show resolved
Hide resolved
src/lib/components/BucketAggregation/BucketAggregationValues.js
Outdated
Show resolved
Hide resolved
| isSelected={isSelected} | ||
| onFilterClicked={onFilterClicked} | ||
| getChildAggCmps={getChildAggCmps} | ||
| valueLabel={label} |
There was a problem hiding this comment.
minor: It is definitely not to be addressed now...but why don't we just name the prop label? The value prefix is unnecessary as the property refers to the label of the ValueElement component. I might miss something though :P But, again, not to be addressed now just curiosity :)
There was a problem hiding this comment.
I didn't want to confuse it with the whole aggregation label (the one you see on top of the filter)
| <Element | ||
| queryString={queryString} | ||
| resetQuery={this.resetQuery} | ||
| extraContent={extraContent} |
There was a problem hiding this comment.
Where is this used? It's a property specific to Overridable?
There was a problem hiding this comment.
no, it is if you want to pass something additional to the EmptyResult, f.e. button to add a new instance or more information. It is more flexible this way than to override the whole component just for this.
There was a problem hiding this comment.
But is not used in the Element, no?
There was a problem hiding this comment.
no it's not used, but I think it should be passed as a prop
There was a problem hiding this comment.
Ok I missed the fact that the Element is overridable :) Then, it looks good!
| <Element | ||
| queryString={queryString} | ||
| resetQuery={this.resetQuery} | ||
| extraContent={extraContent} |
There was a problem hiding this comment.
Ok I missed the fact that the Element is overridable :) Then, it looks good!
3 participants
No description provided.