Skip to content

components: extend Aggregation and EmptyResults interface #128

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

Merged
merged 2 commits into from
Aug 5, 2020
Merged

components: extend Aggregation and EmptyResults interface #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
769dcec
components: extend Aggregation and EmptyResults interface
kpsherva Aug 3, 2020
c093d08
release: v1.0.0-alpha.2
kpsherva Aug 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changes

Version 1.0.0-alpha.2 (released 2020-08-05)

* Add additional props to BucketAggregation component - flexible overrides

Version 1.0.0-alpha.1 (released 2020-05-15)

* Remove 'use strict' directive from the built CommonJS module dist,
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-searchkit",
"version": "1.0.0-alpha.1",
"version": "1.0.0-alpha.2",
"description": "React components to build your search UI application",
"main": "dist/cjs/index.js",
"browser": "dist/cjs/index.js",
Expand Down
9 changes: 8 additions & 1 deletion src/lib/components/BucketAggregation/BucketAggregation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class BucketAggregation extends Component {
};

_renderValues = (resultBuckets, selectedFilters) => {
const { overridableId } = this.props;
const { overridableId, valuesLabels } = this.props;
return (
<BucketAggregationValues
buckets={resultBuckets}
Expand All @@ -37,6 +37,7 @@ class BucketAggregation extends Component {
childAgg={this.agg.childAgg}
onFilterClicked={this.onFilterClicked}
overridableId={overridableId}
valuesLabels={valuesLabels}
/>
);
};
Expand All @@ -59,6 +60,7 @@ class BucketAggregation extends Component {
userSelectionFilters,
resultsAggregations,
overridableId,
...props
} = this.props;
const selectedFilters = this._getSelectedFilters(userSelectionFilters);
const resultBuckets = this._getResultBuckets(resultsAggregations);
Expand All @@ -70,6 +72,7 @@ class BucketAggregation extends Component {
title={this.title}
containerCmp={valuesCmp}
overridableId={overridableId}
{...props}
/>
);
}
Expand All @@ -87,6 +90,10 @@ BucketAggregation.propTypes = {
updateQueryFilters: PropTypes.func.isRequired,
renderValuesContainerElement: PropTypes.func,
renderValueElement: PropTypes.func,
valuesLabels: PropTypes.arrayOf(PropTypes.shape({
value: PropTypes.string.isRequired,
label: PropTypes.string.isRequired,
})),
overridableId: PropTypes.string,
};

Expand Down
28 changes: 20 additions & 8 deletions src/lib/components/BucketAggregation/BucketAggregationValues.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import PropTypes from 'prop-types';
import { Checkbox, List } from 'semantic-ui-react';
import Overridable from 'react-overridable';
import { buildUID } from '../../util';
import _get from 'lodash/get';

class BucketAggregationValues extends Component {
constructor(props) {
Expand Down Expand Up @@ -62,8 +63,7 @@ class BucketAggregationValues extends Component {
};

render() {
const { buckets, selectedFilters, overridableId } = this.props;

const { buckets, selectedFilters, valuesLabels, overridableId } = this.props;
const valuesCmp = buckets.map((bucket) => {
const isSelected = this._isSelected(
this.aggName,
Expand All @@ -75,13 +75,22 @@ class BucketAggregationValues extends Component {
};
const getChildAggCmps = (bucket) =>
this.getChildAggCmps(bucket, selectedFilters);
let label = null;
if(valuesLabels){
label = _get(
valuesLabels.find(e => e.value === bucket.key ),
'label',
bucket.key
);
}
return (
<ValueElement
key={bucket.key}
bucket={bucket}
isSelected={isSelected}
onFilterClicked={onFilterClicked}
getChildAggCmps={getChildAggCmps}
valueLabel={label}
Copy link
Member

@zzacharo zzacharo Aug 4, 2020

Choose a reason for hiding this comment

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

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 :)

Copy link
Contributor Author

@kpsherva kpsherva Aug 4, 2020

Choose a reason for hiding this comment

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

I didn't want to confuse it with the whole aggregation label (the one you see on top of the filter)

overridableId={overridableId}
/>
);
Expand Down Expand Up @@ -116,9 +125,11 @@ const ValueElement = (props) => {
isSelected,
onFilterClicked,
getChildAggCmps,
valueLabel,
overridableId,
} = props;
const label = `${bucket.key} (${bucket.doc_count})`;

const label = valueLabel ? valueLabel : `${bucket.key} (${bucket.doc_count})`;
const childAggCmps = getChildAggCmps(bucket);
return (
<Overridable
Expand All @@ -138,14 +149,15 @@ const ValueElement = (props) => {
);
};

const ContainerElement = ({ valuesCmp, overridableId }) => (
<Overridable
id={buildUID('BucketAggregationContainer.element', overridableId)}
valuesCmp={valuesCmp}
const ContainerElement = ({ valuesCmp, overridableId, valueLabel }) => {
return <Overridable
id={buildUID('BucketAggregationContainer.element', overridableId)}
valuesCmp={valuesCmp}
valueLabel={valueLabel}
>
<List>{valuesCmp}</List>
</Overridable>
);
};

export default Overridable.component(
'BucketAggregationValues',
Expand Down
2 changes: 2 additions & 0 deletions src/lib/components/EmptyResults/EmptyResults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class EmptyResults extends Component {
totalResults,
error,
queryString,
extraContent,
overridableId,
} = this.props;
return (
Expand All @@ -35,6 +36,7 @@ class EmptyResults extends Component {
<Element
queryString={queryString}
resetQuery={this.resetQuery}
extraContent={extraContent}
Copy link
Member

@zzacharo zzacharo Aug 4, 2020

Choose a reason for hiding this comment

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

Where is this used? It's a property specific to Overridable?

Copy link
Contributor Author

@kpsherva kpsherva Aug 4, 2020

Choose a reason for hiding this comment

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

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.

Copy link
Member

@zzacharo zzacharo Aug 4, 2020

Choose a reason for hiding this comment

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

But is not used in the Element, no?

Copy link
Contributor Author

@kpsherva kpsherva Aug 4, 2020

Choose a reason for hiding this comment

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

no it's not used, but I think it should be passed as a prop

Copy link
Member

@zzacharo zzacharo Aug 4, 2020

Choose a reason for hiding this comment

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

Ok I missed the fact that the Element is overridable :) Then, it looks good!

overridableId={overridableId}
/>
</ShouldRender>
Expand Down