From 328033922760e5d14d90f613ccbe2d310a57f8e5 Mon Sep 17 00:00:00 2001
From: Tomasz Knapik <hi@tmkn.org>
Date: Wed, 23 Jan 2019 15:21:33 +0000
Subject: [PATCH] Refactor listing
---
.../src/app/src/components/GroupedListing.js | 124 ++++++++++++++++++
.../src/app/src/components/Listing/index.js | 72 ++++------
.../src/app/src/components/NotesPanel.js | 34 -----
.../src/app/src/containers/ByStatusListing.js | 4 +-
.../app/src/containers/DisplayPanel/index.js | 4 +-
.../NotesPanelItem.js => containers/Note.js} | 12 +-
...SubmissionNotesPanel.js => NoteListing.js} | 32 ++---
.../src/app/src/redux/selectors/notes.js | 21 ++-
.../app/src/redux/selectors/submissions.js | 5 +
9 files changed, 193 insertions(+), 115 deletions(-)
create mode 100644 opentech/static_src/src/app/src/components/GroupedListing.js
delete mode 100644 opentech/static_src/src/app/src/components/NotesPanel.js
rename opentech/static_src/src/app/src/{components/NotesPanelItem.js => containers/Note.js} (67%)
rename opentech/static_src/src/app/src/containers/{SubmissionNotesPanel.js => NoteListing.js} (66%)
diff --git a/opentech/static_src/src/app/src/components/GroupedListing.js b/opentech/static_src/src/app/src/components/GroupedListing.js
new file mode 100644
index 000000000..6da55c346
--- /dev/null
+++ b/opentech/static_src/src/app/src/components/GroupedListing.js
@@ -0,0 +1,124 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+
+import ListingGroup from '@components/ListingGroup';
+import ListingItem from '@components/ListingItem';
+import LoadingPanel from '@components/LoadingPanel';
+
+import '@components/Listing/style.scss';
+
+export default class GroupedListing extends React.Component {
+ static propTypes = {
+ items: PropTypes.array,
+ activeItem: PropTypes.number,
+ isLoading: PropTypes.bool,
+ error: PropTypes.string,
+ groupBy: PropTypes.string,
+ order: PropTypes.arrayOf(PropTypes.string),
+ onItemSelection: PropTypes.func,
+ };
+
+ state = {
+ orderedItems: [],
+ };
+
+ componentDidMount() {
+ this.orderItems();
+ }
+
+ componentDidUpdate(prevProps, prevState) {
+ // Order items
+ if (this.props.items !== prevProps.items) {
+ this.orderItems();
+ }
+
+ const oldItem = prevProps.activeItem
+ const newItem = this.props.activeItem
+
+ // If we have never activated a submission, get the first item
+ if ( !newItem && !oldItem ) {
+ const firstGroup = this.state.orderedItems[0]
+ if ( firstGroup && firstGroup.items[0] ) {
+ this.setState({firstUpdate: false})
+ this.props.onItemSelection(firstGroup.items[0].id)
+ }
+ }
+ }
+
+ renderListItems() {
+ const { isLoading, error, items, onItemSelection, activeItem } = this.props;
+
+ if (isLoading) {
+ return (
+ <div className="listing__list is-loading">
+ <LoadingPanel />
+ </div>
+ )
+ } else if (error) {
+ return (
+ <div className="listing__list is-loading">
+ <p>Something went wrong. Please try again later.</p>
+ <p>{ error }</p>
+ </div>
+ )
+ } else if (items.length === 0) {
+ return (
+ <div className="listing__list is-loading">
+ <p>No results found.</p>
+ </div>
+ )
+ }
+
+ return (
+ <ul className="listing__list">
+ {this.state.orderedItems.map(group => {
+ return (
+ <ListingGroup key={`listing-group-${group.name}`} item={group}>
+ {group.items.map(item => {
+ return <ListingItem
+ selected={!!activeItem && activeItem===item.id}
+ onClick={() => onItemSelection(item.id)}
+ key={`listing-item-${item.id}`}
+ item={item}/>;
+ })}
+ </ListingGroup>
+ );
+ })}
+ </ul>
+ );
+ }
+
+ getGroupedItems() {
+ const { groupBy, items } = this.props;
+
+ return items.reduce((tmpItems, v) => {
+ const groupByValue = v[groupBy];
+ if (!(groupByValue in tmpItems)) {
+ tmpItems[groupByValue] = [];
+ }
+ tmpItems[groupByValue].push({...v});
+ return tmpItems;
+ }, {});
+ }
+
+ orderItems() {
+ const groupedItems = this.getGroupedItems();
+ const { order = [] } = this.props;
+ const leftOverKeys = Object.keys(groupedItems).filter(v => !order.includes(v));
+ this.setState({
+ orderedItems: order.concat(leftOverKeys).filter(key => groupedItems[key] ).map(key => ({
+ name: key,
+ items: groupedItems[key] || []
+ })),
+ });
+ }
+
+ render() {
+ return (
+ <div className="listing">
+ <div className="listing__header"></div>
+ {this.renderListItems()}
+ </div>
+ );
+ }
+}
diff --git a/opentech/static_src/src/app/src/components/Listing/index.js b/opentech/static_src/src/app/src/components/Listing/index.js
index 0296b55c1..bf2cbb850 100644
--- a/opentech/static_src/src/app/src/components/Listing/index.js
+++ b/opentech/static_src/src/app/src/components/Listing/index.js
@@ -1,8 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
-import ListingGroup from '@components/ListingGroup';
-import ListingItem from '@components/ListingItem';
import LoadingPanel from '@components/LoadingPanel';
import './style.scss';
@@ -12,11 +10,14 @@ export default class Listing extends React.Component {
items: PropTypes.array,
activeItem: PropTypes.number,
isLoading: PropTypes.bool,
+ isError: PropTypes.bool,
error: PropTypes.string,
groupBy: PropTypes.string,
order: PropTypes.arrayOf(PropTypes.string),
onItemSelection: PropTypes.func,
shouldSelectFirst: PropTypes.bool,
+ renderItem: PropTypes.func.isRequired,
+ handleRetry: PropTypes.func,
};
static defaultProps = {
@@ -53,71 +54,46 @@ export default class Listing extends React.Component {
}
renderListItems() {
- const { isLoading, error, items, onItemSelection, activeItem } = this.props;
+ const {
+ isError,
+ isLoading,
+ items,
+ renderItem,
+ } = this.props;
if (isLoading) {
return (
<div className="listing__list is-loading">
<LoadingPanel />
</div>
- )
- } else if (error) {
- return (
- <div className="listing__list is-loading">
- <p>Something went wrong. Please try again later.</p>
- <p>{ error }</p>
- </div>
- )
+ );
+ } else if (isError) {
+ return this.renderError();
} else if (items.length === 0) {
return (
<div className="listing__list is-loading">
<p>No results found.</p>
</div>
- )
+ );
}
return (
<ul className="listing__list">
- {this.state.orderedItems.map(group => {
- return (
- <ListingGroup key={`listing-group-${group.name}`} item={group}>
- {group.items.map(item => {
- return <ListingItem
- selected={!!activeItem && activeItem===item.id}
- onClick={() => onItemSelection(item.id)}
- key={`listing-item-${item.id}`}
- item={item}/>;
- })}
- </ListingGroup>
- );
- })}
+ {items.map(v => renderItem(v))}
</ul>
);
}
- getGroupedItems() {
- const { groupBy, items } = this.props;
-
- return items.reduce((tmpItems, v) => {
- const groupByValue = v[groupBy];
- if (!(groupByValue in tmpItems)) {
- tmpItems[groupByValue] = [];
- }
- tmpItems[groupByValue].push({...v});
- return tmpItems;
- }, {});
- }
-
- orderItems() {
- const groupedItems = this.getGroupedItems();
- const { order = [] } = this.props;
- const leftOverKeys = Object.keys(groupedItems).filter(v => !order.includes(v));
- this.setState({
- orderedItems: order.concat(leftOverKeys).filter(key => groupedItems[key] ).map(key => ({
- name: key,
- items: groupedItems[key] || []
- })),
- });
+ renderError = () => {
+ const { handleRetry, error } = this.props;
+ const retryButton = <a onClick={handleRetry}>Refresh</a>;
+ return (
+ <div className="listing__list is-loading">
+ <p>Something went wrong. Please try again later.</p>
+ {error && <p>{error}</p>}
+ {handleRetry && retryButton}
+ </div>
+ );
}
render() {
diff --git a/opentech/static_src/src/app/src/components/NotesPanel.js b/opentech/static_src/src/app/src/components/NotesPanel.js
deleted file mode 100644
index 99bd8f7eb..000000000
--- a/opentech/static_src/src/app/src/components/NotesPanel.js
+++ /dev/null
@@ -1,34 +0,0 @@
-import React from 'react';
-import PropTypes from 'prop-types';
-
-import LoadingPanel from '@components/LoadingPanel';
-
-export default class NotesPanel extends React.Component {
- static propTypes = {
- children: PropTypes.node,
- isLoading: PropTypes.bool.isRequired,
- isErrored: PropTypes.bool.isRequired,
- handleRetry: PropTypes.func.isRequired,
- };
-
- render() {
- const { children, handleRetry, isErrored, isLoading } = this.props;
-
- if (isLoading) {
- return <LoadingPanel />;
- } else if (isErrored) {
- return <>
- <p><strong>Something went wrong!</strong>Sorry we couldn’t load notes</p>
- <a onClick={handleRetry}>Refresh</a>.
- </>;
- } else if (children.length === 0) {
- return <p>There aren’t any notes for this application yet.</p>;
- }
-
- return (
- <ul>
- {children}
- </ul>
- );
- }
-}
diff --git a/opentech/static_src/src/app/src/containers/ByStatusListing.js b/opentech/static_src/src/app/src/containers/ByStatusListing.js
index f33b78e51..c6c2d7052 100644
--- a/opentech/static_src/src/app/src/containers/ByStatusListing.js
+++ b/opentech/static_src/src/app/src/containers/ByStatusListing.js
@@ -2,7 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux'
-import Listing from '@components/Listing';
+import GroupedListing from '@components/GroupedListing';
import {
loadCurrentRound,
setCurrentSubmission,
@@ -50,7 +50,7 @@ class ByStatusListing extends React.Component {
render() {
const { error, submissions, round, setCurrentItem, activeSubmission, shouldSelectFirst} = this.props;
const isLoading = round && round.isFetching
- return <Listing
+ return <GroupedListing
isLoading={isLoading}
error={error}
items={submissions}
diff --git a/opentech/static_src/src/app/src/containers/DisplayPanel/index.js b/opentech/static_src/src/app/src/containers/DisplayPanel/index.js
index ab548c4da..0da90bb5b 100644
--- a/opentech/static_src/src/app/src/containers/DisplayPanel/index.js
+++ b/opentech/static_src/src/app/src/containers/DisplayPanel/index.js
@@ -13,7 +13,7 @@ import {
} from '@selectors/submissions';
import CurrentSubmissionDisplay from '@containers/CurrentSubmissionDisplay'
-import SubmissionNotesPanel from '@containers/SubmissionNotesPanel';
+import NoteListing from '@containers/NoteListing';
import Tabber, {Tab} from '@components/Tabber'
import './style.scss';
@@ -37,7 +37,7 @@ class DisplayPanel extends React.Component {
let tabs = [
<Tab button="Notes" key="note">
- <SubmissionNotesPanel submissionID={submissionID} />
+ <NoteListing submissionID={submissionID} />
</Tab>,
<Tab button="Status" key="status">
<p>Status</p>
diff --git a/opentech/static_src/src/app/src/components/NotesPanelItem.js b/opentech/static_src/src/app/src/containers/Note.js
similarity index 67%
rename from opentech/static_src/src/app/src/components/NotesPanelItem.js
rename to opentech/static_src/src/app/src/containers/Note.js
index 85202c6e6..17152778c 100644
--- a/opentech/static_src/src/app/src/components/NotesPanelItem.js
+++ b/opentech/static_src/src/app/src/containers/Note.js
@@ -1,8 +1,11 @@
import React from 'react';
+import { connect } from 'react-redux';
import PropTypes from 'prop-types';
import moment from 'moment';
-export default class NotesPanelItem extends React.Component {
+import { getNoteOfID } from '@selectors/notes';
+
+class Note extends React.Component {
static propTypes = {
note: PropTypes.shape({
user: PropTypes.string,
@@ -21,4 +24,11 @@ export default class NotesPanelItem extends React.Component {
</div>
);
}
+
}
+
+const mapStateToProps = (state, ownProps) => ({
+ note: getNoteOfID(ownProps.noteID)(state),
+});
+
+export default connect(mapStateToProps)(Note);
diff --git a/opentech/static_src/src/app/src/containers/SubmissionNotesPanel.js b/opentech/static_src/src/app/src/containers/NoteListing.js
similarity index 66%
rename from opentech/static_src/src/app/src/containers/SubmissionNotesPanel.js
rename to opentech/static_src/src/app/src/containers/NoteListing.js
index 6c13bb5fa..7f16d6c64 100644
--- a/opentech/static_src/src/app/src/containers/SubmissionNotesPanel.js
+++ b/opentech/static_src/src/app/src/containers/NoteListing.js
@@ -3,19 +3,19 @@ import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { fetchNotesForSubmission } from '@actions/notes';
-import NotesPanel from '@components/NotesPanel';
-import NotesPanelItem from '@components/NotesPanelItem';
+import Listing from '@components/Listing';
+import Note from '@containers/Note';
import {
getNotesErrorState,
- getNotesForCurrentSubmission,
+ getNoteIDsForSubmissionOfID,
getNotesFetchState,
} from '@selectors/notes';
-class SubmissionNotesPanel extends React.Component {
+class NoteListing extends React.Component {
static propTypes = {
loadNotes: PropTypes.func,
submissionID: PropTypes.number,
- notes: PropTypes.array,
+ noteIDs: PropTypes.array,
isErrored: PropTypes.bool,
isLoading: PropTypes.bool,
};
@@ -39,19 +39,21 @@ class SubmissionNotesPanel extends React.Component {
this.props.loadNotes(this.props.submissionID);
}
+ renderItem = noteID => {
+ return <Note key={`note-${noteID}`} noteID={noteID} />;
+ }
+
render() {
- const { notes } = this.props;
+ const { noteIDs } = this.props;
const passProps = {
isLoading: this.props.isLoading,
- isErrored: this.props.isErrored,
+ isError: this.props.isErrored,
handleRetry: this.handleRetry,
+ renderItem: this.renderItem,
+ items: noteIDs,
};
return (
- <NotesPanel {...passProps}>
- {notes.map(v =>
- <NotesPanelItem key={`note-${v.id}`} note={v} />
- )}
- </NotesPanel>
+ <Listing {...passProps} />
);
}
}
@@ -60,10 +62,10 @@ const mapDispatchToProps = dispatch => ({
loadNotes: submissionID => dispatch(fetchNotesForSubmission(submissionID)),
});
-const mapStateToProps = state => ({
- notes: getNotesForCurrentSubmission(state),
+const mapStateToProps = (state, ownProps) => ({
+ noteIDs: getNoteIDsForSubmissionOfID(ownProps.submissionID)(state),
isLoading: getNotesFetchState(state),
isErrored: getNotesErrorState(state),
});
-export default connect(mapStateToProps, mapDispatchToProps)(SubmissionNotesPanel);
+export default connect(mapStateToProps, mapDispatchToProps)(NoteListing);
diff --git a/opentech/static_src/src/app/src/redux/selectors/notes.js b/opentech/static_src/src/app/src/redux/selectors/notes.js
index e8f068161..cf5ccc2fd 100644
--- a/opentech/static_src/src/app/src/redux/selectors/notes.js
+++ b/opentech/static_src/src/app/src/redux/selectors/notes.js
@@ -1,23 +1,18 @@
import { createSelector } from 'reselect';
-import { getCurrentSubmission } from '@selectors/submissions';
+import { getSubmissionOfID } from '@selectors/submissions';
const getNotes = state => state.notes.byID;
+export const getNoteOfID = (noteID) => createSelector(
+ [getNotes], notes => notes[noteID]
+);
+
export const getNotesFetchState = state => state.notes.isFetching === true;
export const getNotesErrorState = state => state.notes.isErrored === true;
-export const getNotesForCurrentSubmission = createSelector(
- [getCurrentSubmission, getNotes],
- (submission, notes) => {
- if (
- submission === undefined || submission === null ||
- submission.comments === null || submission.comments === undefined
- ) {
- return [];
- }
- return submission.comments.map(commentID => notes[commentID])
- .filter(v => v !== undefined && v !== null)
- }
+export const getNoteIDsForSubmissionOfID = submissionID => createSelector(
+ [getSubmissionOfID(submissionID)],
+ submission => (submission || {}).comments || []
);
diff --git a/opentech/static_src/src/app/src/redux/selectors/submissions.js b/opentech/static_src/src/app/src/redux/selectors/submissions.js
index 09124b689..d012fcb6a 100644
--- a/opentech/static_src/src/app/src/redux/selectors/submissions.js
+++ b/opentech/static_src/src/app/src/redux/selectors/submissions.js
@@ -32,6 +32,10 @@ const getCurrentSubmission = createSelector(
}
);
+const getSubmissionOfID = (submissionID) => createSelector(
+ [getSubmissions], submissions => submissions[submissionID]
+);
+
const getSubmissionLoadingState = state => state.submissions.itemLoading === true;
const getSubmissionErrorState = state => state.submissions.itemLoadingError === true;
@@ -50,4 +54,5 @@ export {
getSubmissionsByRoundLoadingState,
getSubmissionLoadingState,
getSubmissionErrorState,
+ getSubmissionOfID,
};
--
GitLab