Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 37 additions & 0 deletions documentation/components/FacetTabs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#### Examples:


__1:__ A simple example for showing facet as tabs.

```jsx
const DummySearcher = require('../../src/components/DummySearcher').default;
const QueryResponse = require('../../src/api/QueryResponse').default;
const sampleFacets = require('../sampleData/Facets').default;

const queryResponse = new QueryResponse();
queryResponse.facets = [
sampleFacets.regionFacet,
];

<DummySearcher
defaultQueryResponse={queryResponse}
>
<FacetTabs facetName = "region" />
</DummySearcher>
```

__2:__ Example for showing facet tabs by passing in a facet instead of facet name.

```jsx
const sampleFacets = require('../sampleData/Facets').default;

<FacetTabs facet = {sampleFacets.regionFacet} />
```

__3:__ Example for showing facet tabs by passing in a facet and a background color.

```jsx
const sampleFacets = require('../sampleData/Facets').default;

<FacetTabs facet={sampleFacets.regionFacet} backgroundColor='lightgray' />
```
16 changes: 15 additions & 1 deletion src/components/FacetResults.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropTypes from 'prop-types';

import Facet from './Facet';
import SearchFacet from '../api/SearchFacet';
import Configurable from './Configurable';

type FacetResultsProps = {
/** The facet field names that should be displayed as pie charts */
Expand Down Expand Up @@ -42,6 +43,10 @@ type FacetResultsProps = {
* buckets. By default, facets with no buckets will be hidden.
*/
showEmptyFacets: boolean;
/**
* An optional list of facets that should not be shown but are needed for other reasons
*/
hideFacet: Array<string>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add ? to indicate this is optional. Make the prop name plural to indicate it's an array, not a boolean.

Suggested change
hideFacet: Array<string>;
hiddenFacets?: Array<string>;

};

type FacetResultsDefaultProps = {
Expand All @@ -57,6 +62,7 @@ type FacetResultsDefaultProps = {
orderHint: Array<string>;
entityColors: Map<string, string>;
showEmptyFacets: boolean;
hideFacet: Array<string>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename prop to indicate this is an array not a boolean.

Suggested change
hideFacet: Array<string>,
hiddenFacets: Array<string>,

};

/**
Expand All @@ -67,7 +73,7 @@ type FacetResultsDefaultProps = {
* not covered by one of these property's lists will be displayed
* in a standard "More…" list.
*/
export default class FacetResults extends React.Component<FacetResultsDefaultProps, FacetResultsProps, void> {
class FacetResults extends React.Component<FacetResultsDefaultProps, FacetResultsProps, void> {
static defaultProps = {
pieChartFacets: null,
barChartFacets: null,
Expand All @@ -81,6 +87,7 @@ export default class FacetResults extends React.Component<FacetResultsDefaultPro
orderHint: [],
entityColors: new Map(),
showEmptyFacets: false,
hideFacet: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
hideFacet: [],
hiddenFacets: [],

};

static contextTypes = {
Expand Down Expand Up @@ -129,6 +136,11 @@ export default class FacetResults extends React.Component<FacetResultsDefaultPro
}

shouldShow(facet: SearchFacet): boolean {
if (this.props.hideFacet && this.props.hideFacet.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (this.props.hideFacet && this.props.hideFacet.length > 0) {
if (this.props.hiddenFacets && this.props.hiddenFacets.length > 0) {

if (this.props.hideFacet.includes(facet.field)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (this.props.hideFacet.includes(facet.field)) {
if (this.props.hiddenFacets.includes(facet.field)) {

return false;
}
}
if (this.props.showEmptyFacets || (facet && facet.buckets && facet.buckets.length > 0)) {
return true;
}
Expand Down Expand Up @@ -186,3 +198,5 @@ export default class FacetResults extends React.Component<FacetResultsDefaultPro
);
}
}

export default Configurable(FacetResults);
153 changes: 153 additions & 0 deletions src/components/FacetTabs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
// @flow

import React from 'react';
import PropTypes from 'prop-types';

import Configurable from './Configurable';
import SearchFacet from '../api/SearchFacet';
import SearchFacetBucket from '../api/SearchFacetBucket';
import BusyIndicator from './BusyIndicator';

type FacetTabsProps = {
/**
* Name of the facet that should be displayed as Tabs, default is 'table'
*/
facetName: string; //eslint-disable-line
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the eslint-disable-line needed here?

/**
* Optional callback to be used when a tab is clicked instead of default (which applies the facet filter)
*/
handleClick?: () => void;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this callback take information about the facet whose tab is clicked? Perhaps the entire SearchFacetBucket object? How will the callback know what was clicked and what to do?

Oh, I just got down further to where you call this and you are passing the bucket... so you should change the type of this to (bucket: SearchFacetBucket) => void and also add to the comment so someone using this knows what they need to pass in...

/**
* Optional facet that should be used instead of facetName
*/
facet?: SearchFacet | null; //eslint-disable-line
/**
* The color that should be used for the background of facet tabs
*/
backgroundColor?: string;
};

type FacetTabsState = {
tabsList: Array<any>;
};

class FacetTabs extends React.Component<FacetTabsProps, FacetTabsProps, FacetTabsState> {
static contextTypes = {
searcher: PropTypes.any,
};

static defaultProps = {
facetName: 'table',
facet: null,
backgroundColor: '#add8e6',
};

static displayName = 'FacetTabs';

constructor(props: FacetTabsProps) {
super(props);
this.state = {
tabsList: [],
};

(this: any).getTabsFromFacet = this.getTabsFromFacet.bind(this);
(this: any).populateTabs = this.populateTabs.bind(this);
(this: any).handleTabClick = this.handleTabClick.bind(this);
}

componentWillMount() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using componentWillMount

this.populateTabs(this.props);
}

componentWillReceiveProps(newProps: FacetTabsProps) {
this.populateTabs(newProps);
}

//eslint-disable-next-line
numberWithCommas(x) {
return x.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ',');
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can just use x.toLocaleString() to do this without needing a special function.

47626.toLocaleString() gives 47,626.

See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toLocaleString

}

handleTabClick(bucket: SearchFacetBucket, label: string) {
this.context.searcher.addFacetFilter(label, bucket.value, bucket.filter);
}

getTabsFromFacet(facet: SearchFacet) {
const tabs = [];
facet.buckets.forEach((bucket: SearchFacetBucket) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need to have a limit on the number of tabs? What if there are 70 buckets?

const value = bucket.value;
const count = this.numberWithCommas(bucket.count);
if (this.props.handleClick) {
tabs.push(
<div //eslint-disable-line
onClick={() => {
this.props.handleClick(bucket);
}}
className="facet-tab"
>
<div className="facet-tab-text">
{value} <span style={{ color: 'gray' }}>({count})</span>
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're going to have a CSS style for the tab and the text of the tab, you should use another one for the count instead of hardcoding the gray color here.

</div>
</div>,
);
} else {
tabs.push(
<div //eslint-disable-line
onClick={() => {
this.handleTabClick(bucket, facet.label);
}}
className="facet-tab"
>
<div className="facet-tab-text">
{value} <span style={{ color: 'gray' }}>({count})</span>
</div>
</div>,
);
}
});
this.setState({ tabsList: tabs });
Copy link
Contributor

Choose a reason for hiding this comment

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

From react docs, ("Thinking in React")[https://reactjs.org/docs/thinking-in-react.html#step-3-identify-the-minimal-but-complete-representation-of-ui-state]

Ask three questions about each piece of data:

Is it passed in from a parent via props? If so, it probably isn’t state.
Does it remain unchanged over time? If so, it probably isn’t state.
Can you compute it based on any other state or props in your component? If so, it isn’t state.

With that in mind, if you're storing html as state you should reconsider.

}

populateTabs(props: FacetTabsProps) {
const { facetName, facet } = props;

if (facet && facet.buckets) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if the facet had buckets before but doesn't any more (e.g. because of a different search term)? Will the old tabs not be replaced?

this.getTabsFromFacet(facet);
} else if (facetName) {
const searcher = this.context.searcher;
if (searcher && searcher.state.response) {
if (searcher.state.response.facets) {
searcher.state.response.facets.forEach((currentFacet: SearchFacet) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is really a find() operation on the array.... it would be clearer to call find() instead of forEach()....

if (currentFacet.field === facetName && currentFacet.buckets) {
this.getTabsFromFacet(currentFacet);
}
});
}
}
}
}

render() {
const tabData = this.state.tabsList;
const tabs =
Copy link
Contributor

Choose a reason for hiding this comment

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

Pull tabs out into renderTabs

tabData.length > 0 ? (
<div
className="facet-tab-container"
style={{ backgroundColor: `${this.props.backgroundColor}` }}
>
{tabData}
</div>
) : (
<BusyIndicator
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if there are just no buckets for the facet? I don't think we want to show the spinny in that case...

show
type="spinny"
message="Loading Facet Tabs..."
messageStyle={{ fontWeight: 600, color: '#2f75b0' }}
positionMessageRight
/>
);
return <div>{tabs}</div>;
}
}

export default Configurable(FacetTabs);
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export Facet from './components/Facet';
export FacetInsights from './components/FacetInsights';
export FacetResults from './components/FacetResults';
export FacetSearchBar from './components/FacetSearchBar';
export FacetTabs from './components/FacetTabs';
export FormattedDate from './components/FormattedDate';
export GridLayout from './components/GridLayout';
export Header360 from './components/Header360';
Expand Down
1 change: 1 addition & 0 deletions styleguide.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ module.exports = {
'src/components/FacetInsights.js',
'src/components/FacetResults.js',
'src/components/FacetSearchBar.js',
'src/components/FacetTabs.js',
'src/components/HierarchicalFacetContents.js',
'src/components/ListWithBarsFacetContents.js',
'src/components/MapFacetContents.js',
Expand Down