From b63fce4ebc9ae7dc001f05d6e557e206a181a199 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 15 Aug 2017 16:58:23 +0700 Subject: [PATCH 01/40] Optimize Slot rendering --- client/coral-framework/components/Slot.js | 49 ++++++++++++++++++----- client/coral-framework/helpers/plugins.js | 6 +++ 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/client/coral-framework/components/Slot.js b/client/coral-framework/components/Slot.js index b1df6647a..f86d44eb1 100644 --- a/client/coral-framework/components/Slot.js +++ b/client/coral-framework/components/Slot.js @@ -4,19 +4,48 @@ import styles from './Slot.css'; import {connect} from 'react-redux'; import {getSlotElements} from 'coral-framework/helpers/plugins'; import omit from 'lodash/omit'; +import union from 'lodash/union'; +import isEqual from 'lodash/isEqual'; -function Slot ({fill, inline = false, className, reduxState, defaultComponent: DefaultComponent, ...rest}) { - let children = getSlotElements(fill, reduxState, rest); - const pluginConfig = reduxState.config.pluginConfig || {}; - if (children.length === 0 && DefaultComponent) { - children = ; +class Slot extends React.Component { + shouldComponentUpdate(next) { + + // Prevent Slot from rerendering when only reduxState has changed and + // it does not result in a change of slot children. + const keys = union(Object.keys(this.props), Object.keys(next)); + const changes = keys.filter((key) => this.props[key] !== next[key]); + if (changes.length === 1 && changes[0] === 'reduxState') { + const prevChildrenUuid = this.getChildren(this.props).map((child) => child.type.talkUuid); + const nextChildrenUuid = this.getChildren(next).map((child) => child.type.talkUuid); + return !isEqual(prevChildrenUuid, nextChildrenUuid); + } + + // Prevent Slot from rerendering when no props has shallowly changed. + return changes.length !== 0; } - return ( -
- {children} -
- ); + getSlotProps({fill: _a, inline: _b, className: _c, reduxState: _d, defaultComponent_: _e, ...rest} = this.props) { + return rest; + } + + getChildren(props = this.props) { + return getSlotElements(props.fill, props.reduxState, this.getSlotProps(props)); + } + + render() { + const {inline = false, className, reduxState, defaultComponent: DefaultComponent} = this.props; + let children = this.getChildren(); + const pluginConfig = reduxState.config.pluginConfig || {}; + if (children.length === 0 && DefaultComponent) { + children = ; + } + + return ( +
+ {children} +
+ ); + } } Slot.propTypes = { diff --git a/client/coral-framework/helpers/plugins.js b/client/coral-framework/helpers/plugins.js index b2ca78142..7bebaa1f6 100644 --- a/client/coral-framework/helpers/plugins.js +++ b/client/coral-framework/helpers/plugins.js @@ -9,6 +9,7 @@ import {loadTranslations} from 'coral-framework/services/i18n'; import {injectReducers} from 'coral-framework/services/store'; import camelize from './camelize'; import plugins from 'pluginsConfig'; +import uuid from 'uuid/v4'; export function getSlotComponents(slot, reduxState, props = {}) { const pluginConfig = reduxState.config.plugin_config || {}; @@ -100,7 +101,12 @@ function addMetaDataToSlotComponents() { const slots = plugin.module.slots; slots && Object.keys(slots).forEach((slot) => { slots[slot].forEach((component) => { + + // Attach plugin name to the component component.talkPluginName = plugin.name; + + // Attach uuid to the component + component.talkUuid = uuid(); }); }); }); From 4cfdb6574bfc651a8cf829ee387c59ec7597dc60 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 15 Aug 2017 21:23:39 +0700 Subject: [PATCH 02/40] withQuery props should be immutable --- client/coral-framework/hocs/withQuery.js | 129 ++++++++++++++--------- 1 file changed, 82 insertions(+), 47 deletions(-) diff --git a/client/coral-framework/hocs/withQuery.js b/client/coral-framework/hocs/withQuery.js index adb67b7b3..2ddca7c7e 100644 --- a/client/coral-framework/hocs/withQuery.js +++ b/client/coral-framework/hocs/withQuery.js @@ -46,6 +46,7 @@ export default (document, config = {}) => (WrappedComponent) => { // Lazily resolve fragments from graphRegistry to support circular dependencies. memoized = null; lastNetworkStatus = null; + data = null; emitWhenNeeded(data) { const {variables, networkStatus} = data; @@ -60,59 +61,93 @@ export default (document, config = {}) => (WrappedComponent) => { this.context.eventEmitter.emit(`query.${name}.${status}`, {variables, data: root}); } + nextData(data) { + this.emitWhenNeeded(data); + + // If data was previously set, we update it in a immutable way. + if (this.data) { + if (this.data.networkStatus !== data.networkStatus || + this.data.loading !== data.loading || + this.data.error !== data.error || + this.data.variables !== data.variables) { + this.data = { + ...this.data, + error: data.error, + networkStatus: data.networkStatus, + loading: data.loading, + variables: data.variables, + }; + } + } + else { + + // Set data for the first time. + this.data = { + error: data.error, + variables: data.variables, + networkStatus: data.networkStatus, + loading: data.loading, + startPolling: data.startPolling, + stopPolling: data.stopPolling, + refetch: data.refetch, + updateQuery: data.updateQuery, + subscribeToMore: (stmArgs) => { + + // Resolve document fragments before passing it to `apollo-client`. + return data.subscribeToMore({ + ...stmArgs, + document: resolveFragments(stmArgs.document), + onError: (err) => { + if (stmArgs.onErr) { + return stmArgs.onErr(err); + } + throw err; + }, + }); + }, + fetchMore: (lmArgs) => { + const fetchName = getDefinitionName(lmArgs.query); + this.context.eventEmitter.emit( + `query.${name}.fetchMore.${fetchName}.begin`, + {variables: lmArgs.variables}); + + // Resolve document fragments before passing it to `apollo-client`. + return data.fetchMore({ + ...lmArgs, + query: resolveFragments(lmArgs.query), + }) + .then((res) => { + this.context.eventEmitter.emit( + `query.${name}.fetchMore.${fetchName}.success`, + {variables: lmArgs.variables, data: res.data}); + return Promise.resolve(res); + }) + .catch((err) => { + this.context.eventEmitter.emit( + `query.${name}.fetchMore.${fetchName}.error`, + {variables: lmArgs.variables, error: err}); + throw err; + }); + }, + }; + } + return this.data; + } + wrappedConfig = { ...config, options: config.options || {}, props: (args) => { - this.emitWhenNeeded(args.data); + const nextData = this.nextData(args.data); + const {root} = separateDataAndRoot(args.data); + if (config.props) { - const wrappedArgs = { - ...args, - data: { - ...args.data, - subscribeToMore: (stmArgs) => { + // Custom props, in this case we just pass the wrapped args to it. + return config.props({...args, data: {...args.data, ...nextData}}); + } - // Resolve document fragments before passing it to `apollo-client`. - return args.data.subscribeToMore({ - ...stmArgs, - document: resolveFragments(stmArgs.document), - onError: (err) => { - if (stmArgs.onErr) { - return stmArgs.onErr(err); - } - throw err; - }, - }); - }, - fetchMore: (lmArgs) => { - const fetchName = getDefinitionName(lmArgs.query); - this.context.eventEmitter.emit( - `query.${name}.fetchMore.${fetchName}.begin`, - {variables: lmArgs.variables}); - - // Resolve document fragments before passing it to `apollo-client`. - return args.data.fetchMore({ - ...lmArgs, - query: resolveFragments(lmArgs.query), - }) - .then((res) => { - this.context.eventEmitter.emit( - `query.${name}.fetchMore.${fetchName}.success`, - {variables: lmArgs.variables, data: res.data}); - return Promise.resolve(res); - }) - .catch((err) => { - this.context.eventEmitter.emit( - `query.${name}.fetchMore.${fetchName}.error`, - {variables: lmArgs.variables, error: err}); - throw err; - }); - }, - }, - }; - return config.props - ? config.props(wrappedArgs) - : separateDataAndRoot(wrappedArgs.data); + // Return our wrapped data with a separated root. + return {...args, data: nextData, root}; }, }; From 2b2e2f9b5f5fef5a67f6289e9ae2d4f7189933ce Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 15 Aug 2017 21:32:44 +0700 Subject: [PATCH 03/40] ClickOutside should only toggle action when open --- client/coral-embed-stream/src/components/Toggleable.js | 4 +++- client/talk-plugin-flags/components/FlagButton.js | 4 +++- .../client/components/PermalinkButton.js | 8 +++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/client/coral-embed-stream/src/components/Toggleable.js b/client/coral-embed-stream/src/components/Toggleable.js index 49f2a701b..a65e8212c 100644 --- a/client/coral-embed-stream/src/components/Toggleable.js +++ b/client/coral-embed-stream/src/components/Toggleable.js @@ -19,7 +19,9 @@ export default class Toggleable extends React.Component { } close = () => { - this.setState({isOpen: false}); + if (this.state.isOpen) { + this.setState({isOpen: false}); + } } render() { diff --git a/client/talk-plugin-flags/components/FlagButton.js b/client/talk-plugin-flags/components/FlagButton.js index 85ceed0ea..1d8438ba0 100644 --- a/client/talk-plugin-flags/components/FlagButton.js +++ b/client/talk-plugin-flags/components/FlagButton.js @@ -133,7 +133,9 @@ export default class FlagButton extends Component { } handleClickOutside = () => { - this.closeMenu(); + if (this.state.showMenu) { + this.closeMenu(); + } } render () { diff --git a/plugins/talk-plugin-permalink/client/components/PermalinkButton.js b/plugins/talk-plugin-permalink/client/components/PermalinkButton.js index 27cbf9188..e2536f86f 100644 --- a/plugins/talk-plugin-permalink/client/components/PermalinkButton.js +++ b/plugins/talk-plugin-permalink/client/components/PermalinkButton.js @@ -28,9 +28,11 @@ export default class PermalinkButton extends React.Component { } handleClickOutside = () => { - this.setState({ - popoverOpen: false - }); + if (this.state.popoverOpen) { + this.setState({ + popoverOpen: false + }); + } } copyPermalink = () => { From 33aea66ff774bf5ff48938eb4851018765e04050 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Tue, 15 Aug 2017 21:56:22 +0700 Subject: [PATCH 04/40] Refactor --- client/coral-framework/components/Slot.js | 7 +++---- client/coral-framework/utils/index.js | 6 ++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/client/coral-framework/components/Slot.js b/client/coral-framework/components/Slot.js index f86d44eb1..0b0f4d325 100644 --- a/client/coral-framework/components/Slot.js +++ b/client/coral-framework/components/Slot.js @@ -4,16 +4,15 @@ import styles from './Slot.css'; import {connect} from 'react-redux'; import {getSlotElements} from 'coral-framework/helpers/plugins'; import omit from 'lodash/omit'; -import union from 'lodash/union'; import isEqual from 'lodash/isEqual'; +import {getShallowChanges} from 'coral-framework/utils'; class Slot extends React.Component { shouldComponentUpdate(next) { // Prevent Slot from rerendering when only reduxState has changed and // it does not result in a change of slot children. - const keys = union(Object.keys(this.props), Object.keys(next)); - const changes = keys.filter((key) => this.props[key] !== next[key]); + const changes = getShallowChanges(this.props, next); if (changes.length === 1 && changes[0] === 'reduxState') { const prevChildrenUuid = this.getChildren(this.props).map((child) => child.type.talkUuid); const nextChildrenUuid = this.getChildren(next).map((child) => child.type.talkUuid); @@ -49,7 +48,7 @@ class Slot extends React.Component { } Slot.propTypes = { - fill: React.PropTypes.string + fill: React.PropTypes.string.isRequired }; const mapStateToProps = (state) => ({ diff --git a/client/coral-framework/utils/index.js b/client/coral-framework/utils/index.js index 63dfd7dc4..2b4b6378f 100644 --- a/client/coral-framework/utils/index.js +++ b/client/coral-framework/utils/index.js @@ -1,5 +1,6 @@ import {gql} from 'react-apollo'; import t from 'coral-framework/services/i18n'; +import union from 'lodash/union'; import {capitalize} from 'coral-framework/helpers/strings'; export const getTotalActionCount = (type, comment) => { @@ -184,3 +185,8 @@ export function getSlotFragmentSpreads(slots, resource) { export function isCommentActive(commentStatus) { return ['NONE', 'ACCEPTED'].indexOf(commentStatus) >= 0; } + +export function getShallowChanges(a, b) { + return union(Object.keys(a), Object.keys(b)) + .filter((key) => a[key] !== b[key]); +} From 9ce6ab108a3b7c2ffe65fbbbdeaa73108ea8eafd Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 16 Aug 2017 22:07:25 +0700 Subject: [PATCH 05/40] Refactor StreamTabPanel and use hoistStatics for all hocs --- .../src/components/Comment.js | 2 +- .../src/components/Stream.js | 131 ++++++------------ .../src/components/StreamTabPanel.js | 23 +++ .../src/containers/Stream.js | 8 +- .../src/containers/StreamTabPanel.js | 84 +++++++++++ .../hocs/withCopyToClipboard.js | 5 +- client/coral-framework/hocs/withEmit.js | 7 +- client/coral-framework/hocs/withFragments.js | 24 +++- client/coral-framework/hocs/withMutation.js | 5 +- client/coral-framework/hocs/withQuery.js | 5 +- plugin-api/beta/client/hocs/withTags.js | 5 +- 11 files changed, 195 insertions(+), 104 deletions(-) create mode 100644 client/coral-embed-stream/src/components/StreamTabPanel.js create mode 100644 client/coral-embed-stream/src/containers/StreamTabPanel.js diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index 6c231f4ae..b8063858d 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -72,7 +72,7 @@ const ActionButton = ({children}) => { ); }; -export default class Comment extends React.Component { +export default class Comment extends React.PureComponent { constructor(props) { super(props); diff --git a/client/coral-embed-stream/src/components/Stream.js b/client/coral-embed-stream/src/components/Stream.js index 7b9e5f566..a9ea98a68 100644 --- a/client/coral-embed-stream/src/components/Stream.js +++ b/client/coral-embed-stream/src/components/Stream.js @@ -10,16 +10,16 @@ import {ModerationLink} from 'talk-plugin-moderation'; import RestrictedMessageBox from 'coral-framework/components/RestrictedMessageBox'; import t, {timeago} from 'coral-framework/services/i18n'; -import {getSlotComponents} from 'coral-framework/helpers/plugins'; import CommentBox from 'talk-plugin-commentbox/CommentBox'; import QuestionBox from 'talk-plugin-questionbox/QuestionBox'; import {isCommentActive} from 'coral-framework/utils'; -import {Button, TabBar, Tab, TabCount, TabContent, TabPane} from 'coral-ui'; +import {Button, Tab, TabCount, TabPane} from 'coral-ui'; import cn from 'classnames'; import {getTopLevelParent, attachCommentToParent} from '../graphql/utils'; import AllCommentsPane from './AllCommentsPane'; import AutomaticAssetClosure from '../containers/AutomaticAssetClosure'; +import StreamTabPanel from '../containers/StreamTabPanel'; import styles from './Stream.css'; @@ -35,44 +35,9 @@ class Stream extends React.Component { componentWillReceiveProps(next) { // Keep comment box when user was live suspended, banned, ... - if (!this.userIsDegraged(this.props) && this.userIsDegraged(next)) { + if (!this.props.userIsDegraged && next.userIsDegraged) { this.setState({keepCommentBox: true}); } - - this.fallbackAllTab(next); - } - - componentDidMount() { - this.fallbackAllTab(); - } - - fallbackAllTab(props = this.props) { - if (props.activeStreamTab !== 'all') { - const slotPlugins = this.getSlotComponents('streamTabs', props).map((c) => c.talkPluginName); - if (slotPlugins.indexOf(props.activeStreamTab) === -1) { - props.setActiveStreamTab('all'); - } - } - } - - getSlotProps({data, root, root: {asset}} = this.props) { - return {data, root, asset}; - } - - getSlotComponents(slot, props = this.props) { - return getSlotComponents(slot, props.reduxState, this.getSlotProps(props)); - } - - setActiveReplyBox = (id) => { - if (!this.props.auth.user) { - this.props.showSignInDialog(); - } else { - this.props.setActiveReplyBox(id); - } - }; - - userIsDegraged({auth: {user}} = this.props) { - return !can(user, 'INTERACT_WITH_COMMUNITY'); } render() { @@ -99,7 +64,7 @@ class Stream extends React.Component { loadMoreComments, viewAllComments, auth: {loggedIn, user}, - editName + editName, } = this.props; const {keepCommentBox} = this.state; const open = !asset.isClosed; @@ -133,7 +98,7 @@ class Stream extends React.Component { }; const showCommentBox = loggedIn && ((!banned && !temporarilySuspended && !highlightedComment) || keepCommentBox); - const slotProps = this.getSlotProps(); + const slotProps = {data, root, asset}; if (!comment && !comments) { console.error('Talk: No comments came back from the graph given that query. Please, check the query params.'); @@ -247,55 +212,49 @@ class Stream extends React.Component { {...slotProps} /> - - {this.getSlotComponents('streamTabs').map((PluginComponent) => ( - - + + All Comments {totalCommentCount} - ))} - - All Comments {totalCommentCount} - - - - {this.getSlotComponents('streamTabPanes').map((PluginComponent) => ( - - + - ))} - - - - + } + sub + /> } diff --git a/client/coral-embed-stream/src/components/StreamTabPanel.js b/client/coral-embed-stream/src/components/StreamTabPanel.js new file mode 100644 index 000000000..c5a292179 --- /dev/null +++ b/client/coral-embed-stream/src/components/StreamTabPanel.js @@ -0,0 +1,23 @@ +import React from 'react'; +import {TabBar, TabContent} from 'coral-ui'; + +class StreamTabPanel extends React.Component { + + render() { + const {activeTab, setActiveTab, appendTabs, appendTabPanes, pluginTabElements, pluginTabPaneElements, sub} = this.props; + return ( +
+ + {pluginTabElements} + {appendTabs} + + + {pluginTabPaneElements} + {appendTabPanes} + +
+ ); + } +} + +export default StreamTabPanel; diff --git a/client/coral-embed-stream/src/containers/Stream.js b/client/coral-embed-stream/src/containers/Stream.js index 451b115a9..acc178261 100644 --- a/client/coral-embed-stream/src/containers/Stream.js +++ b/client/coral-embed-stream/src/containers/Stream.js @@ -16,6 +16,7 @@ import Comment from './Comment'; import {withFragments, withEmit} from 'coral-framework/hocs'; import {getDefinitionName, getSlotFragmentSpreads} from 'coral-framework/utils'; import {Spinner} from 'coral-ui'; +import {can} from 'coral-framework/services/perms'; import { findCommentInEmbedQuery, insertCommentIntoEmbedQuery, @@ -23,7 +24,6 @@ import { insertFetchedCommentsIntoEmbedQuery, nest, } from '../graphql/utils'; -import omit from 'lodash/omit'; const {showSignInDialog, editName} = authActions; const {addNotification} = notificationActions; @@ -140,6 +140,10 @@ class StreamContainer extends React.Component { clearInterval(this.countPoll); } + userIsDegraged({auth: {user}} = this.props) { + return !can(user, 'INTERACT_WITH_COMMUNITY'); + } + render() { if (this.props.refetching) { return ; @@ -149,6 +153,7 @@ class StreamContainer extends React.Component { loadMore={this.loadMore} loadMoreComments={this.loadMoreComments} loadNewReplies={this.loadNewReplies} + userIsDegraged={this.userIsDegraged()} />; } } @@ -310,7 +315,6 @@ const mapStateToProps = (state) => ({ previousStreamTab: state.stream.previousTab, commentClassNames: state.stream.commentClassNames, pluginConfig: state.config.plugin_config, - reduxState: omit(state, 'apollo'), }); const mapDispatchToProps = (dispatch) => diff --git a/client/coral-embed-stream/src/containers/StreamTabPanel.js b/client/coral-embed-stream/src/containers/StreamTabPanel.js new file mode 100644 index 000000000..574c4c881 --- /dev/null +++ b/client/coral-embed-stream/src/containers/StreamTabPanel.js @@ -0,0 +1,84 @@ +import React from 'react'; +import StreamTabPanel from '../components/StreamTabPanel'; +import {connect} from 'react-redux'; +import omit from 'lodash/omit'; +import {getSlotComponents} from 'coral-framework/helpers/plugins'; +import {Tab, TabPane} from 'coral-ui'; +import {getShallowChanges} from 'coral-framework/utils'; +import isEqual from 'lodash/isEqual'; + +class StreamTabPanelContainer extends React.Component { + + componentDidMount() { + this.fallbackAllTab(); + } + + componentWillReceiveProps(next) { + this.fallbackAllTab(next); + } + + shouldComponentUpdate(next) { + + // Prevent Slot from rerendering when only reduxState has changed and + // it does not result in a change of slot children. + const changes = getShallowChanges(this.props, next); + if (changes.length === 1 && changes[0] === 'reduxState') { + const prevUuid = this.getSlotComponents(this.props.tabSlot, this.props).map((cmp) => cmp.talkUuid); + const nextUuid = this.getSlotComponents(next.tabSlot, next).map((cmp) => cmp.talkUuid); + return !isEqual(prevUuid, nextUuid); + } + + // Prevent Slot from rerendering when no props has shallowly changed. + return changes.length !== 0; + } + + fallbackAllTab(props = this.props) { + if (props.activeTab !== props.fallbackTab) { + const slotPlugins = this.getSlotComponents(props.tabSlot, props).map((c) => c.talkPluginName); + if (slotPlugins.indexOf(props.activeTab) === -1) { + props.setActiveTab(props.fallbackTab); + } + } + } + + getSlotComponents(slot, props = this.props) { + return getSlotComponents(slot, props.reduxState, props.slotProps); + } + + getPluginTabElements(props = this.props) { + return this.getSlotComponents(props.tabSlot).map((PluginComponent) => ( + + + + )); + } + + getPluginTabPaneElements(props = this.props) { + return this.getSlotComponents(props.tabPaneSlot).map((PluginComponent) => ( + + + + )); + } + + render() { + return ( + + ); + } +} + +const mapStateToProps = (state) => ({ + reduxState: omit(state, 'apollo'), +}); + +export default connect(mapStateToProps, null)(StreamTabPanelContainer); diff --git a/client/coral-framework/hocs/withCopyToClipboard.js b/client/coral-framework/hocs/withCopyToClipboard.js index 9e8046802..4de606c40 100644 --- a/client/coral-framework/hocs/withCopyToClipboard.js +++ b/client/coral-framework/hocs/withCopyToClipboard.js @@ -1,8 +1,9 @@ import React from 'react'; import ReactDOM from 'react-dom'; import Clipboard from 'clipboard'; +import hoistStatics from 'recompose/hoistStatics'; -export default (WrappedComponent) => { +export default hoistStatics((WrappedComponent) => { class WithCopyToClipboard extends React.Component { componentDidMount() { const clipboard = new Clipboard(ReactDOM.findDOMNode(this)); @@ -26,4 +27,4 @@ export default (WrappedComponent) => { } return WithCopyToClipboard; -}; +}); diff --git a/client/coral-framework/hocs/withEmit.js b/client/coral-framework/hocs/withEmit.js index 3a3216c8a..22d98bb27 100644 --- a/client/coral-framework/hocs/withEmit.js +++ b/client/coral-framework/hocs/withEmit.js @@ -1,11 +1,12 @@ import React from 'react'; -const PropTypes = require('prop-types'); +import hoistStatics from 'recompose/hoistStatics'; +import PropTypes from 'prop-types'; /** * WithEmit provides a property `emit: (eventName, value)` * to the wrapped component. */ -export default (WrappedComponent) => { +export default hoistStatics((WrappedComponent) => { class WithEmit extends React.Component { static contextTypes = { eventEmitter: PropTypes.object, @@ -24,4 +25,4 @@ export default (WrappedComponent) => { } return WithEmit; -}; +}); diff --git a/client/coral-framework/hocs/withFragments.js b/client/coral-framework/hocs/withFragments.js index 62e96444c..c422f8705 100644 --- a/client/coral-framework/hocs/withFragments.js +++ b/client/coral-framework/hocs/withFragments.js @@ -1,5 +1,21 @@ // TODO: revisit `filtering` after https://github.com/apollographql/graphql-anywhere/issues/38. -export default (fragments) => (BaseComponent) => { - BaseComponent.fragments = fragments; - return BaseComponent; -}; + +import React from 'react'; +import {resolveFragments} from 'coral-framework/services/graphqlRegistry'; +import mapValues from 'lodash/mapValues'; +import hoistStatics from 'recompose/hoistStatics'; + +export default (fragments) => hoistStatics((BaseComponent) => { + class WithFragments extends React.Component { + fragments = mapValues(fragments, (val) => resolveFragments(val)); + + render() { + return ; + } + } + + WithFragments.fragments = fragments; + return WithFragments; +}); diff --git a/client/coral-framework/hocs/withMutation.js b/client/coral-framework/hocs/withMutation.js index 7e2c3d09d..bb2321d11 100644 --- a/client/coral-framework/hocs/withMutation.js +++ b/client/coral-framework/hocs/withMutation.js @@ -8,6 +8,7 @@ import {getMutationOptions, resolveFragments} from 'coral-framework/services/gra import {getDefinitionName, getResponseErrors} from '../utils'; import PropTypes from 'prop-types'; import t from 'coral-framework/services/i18n'; +import hoistStatics from 'recompose/hoistStatics'; class ResponseErrors extends Error { constructor(errors) { @@ -30,7 +31,7 @@ class ResponseError { * Exports a HOC with the same signature as `graphql`, that will * apply mutation options registered in the graphRegistry. */ -export default (document, config = {}) => (WrappedComponent) => { +export default (document, config = {}) => hoistStatics((WrappedComponent) => { config = { ...config, options: config.options || {}, @@ -147,4 +148,4 @@ export default (document, config = {}) => (WrappedComponent) => { return ; } }; -}; +}); diff --git a/client/coral-framework/hocs/withQuery.js b/client/coral-framework/hocs/withQuery.js index d98400419..f19c3209e 100644 --- a/client/coral-framework/hocs/withQuery.js +++ b/client/coral-framework/hocs/withQuery.js @@ -3,6 +3,7 @@ import {graphql} from 'react-apollo'; import {getQueryOptions, resolveFragments} from 'coral-framework/services/graphqlRegistry'; import {getDefinitionName, separateDataAndRoot, getResponseErrors} from '../utils'; import PropTypes from 'prop-types'; +import hoistStatics from 'recompose/hoistStatics'; const withSkipOnErrors = (reducer) => (prev, action, ...rest) => { if (action.type === 'APOLLO_MUTATION_RESULT' && getResponseErrors(action.result)) { @@ -35,7 +36,7 @@ function networkStatusToString(networkStatus) { * Exports a HOC with the same signature as `graphql`, that will * apply query options registered in the graphRegistry. */ -export default (document, config = {}) => (WrappedComponent) => { +export default (document, config = {}) => hoistStatics((WrappedComponent) => { const name = getDefinitionName(document); return class WithQuery extends React.Component { @@ -190,4 +191,4 @@ export default (document, config = {}) => (WrappedComponent) => { return ; } }; -}; +}); diff --git a/plugin-api/beta/client/hocs/withTags.js b/plugin-api/beta/client/hocs/withTags.js index b0a74f4d7..428c3f43f 100644 --- a/plugin-api/beta/client/hocs/withTags.js +++ b/plugin-api/beta/client/hocs/withTags.js @@ -8,8 +8,9 @@ import {withAddTag, withRemoveTag} from 'coral-framework/graphql/mutations'; import withFragments from 'coral-framework/hocs/withFragments'; import {addNotification} from 'coral-framework/actions/notification'; import {forEachError, isTagged} from 'coral-framework/utils'; +import hoistStatics from 'recompose/hoistStatics'; -export default (tag) => (WrappedComponent) => { +export default (tag) => hoistStatics((WrappedComponent) => { if (typeof tag !== 'string') { console.error('Tag must be a valid string'); return null; @@ -109,4 +110,4 @@ export default (tag) => (WrappedComponent) => { WithTags.displayName = `WithTags(${getDisplayName(WrappedComponent)})`; return enhance(WithTags); -}; +}); From 4aa6b530228352dbcd3f6387fcd88582d7866675 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 16 Aug 2017 22:32:44 +0700 Subject: [PATCH 06/40] PropTypes for StreamTabPanel --- .../src/components/Stream.js | 4 +-- .../src/components/StreamTabPanel.js | 24 ++++++++++++---- .../src/containers/StreamTabPanel.js | 28 +++++++++++++++++-- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/client/coral-embed-stream/src/components/Stream.js b/client/coral-embed-stream/src/components/Stream.js index a9ea98a68..26b054e0d 100644 --- a/client/coral-embed-stream/src/components/Stream.js +++ b/client/coral-embed-stream/src/components/Stream.js @@ -220,12 +220,12 @@ class Stream extends React.Component { tabPaneSlot={'streamTabPanes'} slotProps={slotProps} appendTabs={ - + All Comments {totalCommentCount} } appendTabPanes={ - + - {pluginTabElements} - {appendTabs} + {tabs} - {pluginTabPaneElements} - {appendTabPanes} + {tabPanes} ); } } +StreamTabPanel.propTypes = { + activeTab: PropTypes.string.isRequired, + setActiveTab: PropTypes.func.isRequired, + tabs: PropTypes.oneOfType([ + PropTypes.element, + PropTypes.arrayOf(PropTypes.element) + ]), + tabPanes: PropTypes.oneOfType([ + PropTypes.element, + PropTypes.arrayOf(PropTypes.element) + ]), + className: PropTypes.string, + sub: PropTypes.bool, +}; + export default StreamTabPanel; diff --git a/client/coral-embed-stream/src/containers/StreamTabPanel.js b/client/coral-embed-stream/src/containers/StreamTabPanel.js index 574c4c881..4a313d2b4 100644 --- a/client/coral-embed-stream/src/containers/StreamTabPanel.js +++ b/client/coral-embed-stream/src/containers/StreamTabPanel.js @@ -6,6 +6,7 @@ import {getSlotComponents} from 'coral-framework/helpers/plugins'; import {Tab, TabPane} from 'coral-ui'; import {getShallowChanges} from 'coral-framework/utils'; import isEqual from 'lodash/isEqual'; +import PropTypes from 'prop-types'; class StreamTabPanelContainer extends React.Component { @@ -69,14 +70,35 @@ class StreamTabPanelContainer extends React.Component { render() { return ( ); } } +StreamTabPanelContainer.propTypes = { + activeTab: PropTypes.string.isRequired, + setActiveTab: PropTypes.func.isRequired, + appendTabs: PropTypes.oneOfType([ + PropTypes.element, + PropTypes.arrayOf(PropTypes.element) + ]), + appendTabPanes: PropTypes.oneOfType([ + PropTypes.element, + PropTypes.arrayOf(PropTypes.element) + ]), + fallbackTab: PropTypes.string.isRequired, + tabSlot: PropTypes.string.isRequired, + tabPaneSlot: PropTypes.string.isRequired, + className: PropTypes.string, + sub: PropTypes.bool, +}; + const mapStateToProps = (state) => ({ reduxState: omit(state, 'apollo'), }); From ae81d5cb3c703d13832c508d79b42e2c7fac9d08 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 16 Aug 2017 22:43:48 +0700 Subject: [PATCH 07/40] Optimize IfSlotIsX rendering --- .../components/IfSlotIsEmpty.js | 35 +++++++++++++++---- .../components/IfSlotIsNotEmpty.js | 35 +++++++++++++++---- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/client/coral-framework/components/IfSlotIsEmpty.js b/client/coral-framework/components/IfSlotIsEmpty.js index 424a59bc7..e9211fc57 100644 --- a/client/coral-framework/components/IfSlotIsEmpty.js +++ b/client/coral-framework/components/IfSlotIsEmpty.js @@ -3,13 +3,36 @@ import {connect} from 'react-redux'; import {isSlotEmpty} from 'coral-framework/helpers/plugins'; import PropTypes from 'prop-types'; import omit from 'lodash/omit'; +import {getShallowChanges} from 'coral-framework/utils'; -function IfSlotIsEmpty({slot, className, reduxState, component: Component = 'div', children, ...rest}) { - return ( - - {isSlotEmpty(slot, reduxState, rest) ? children : null} - - ); +class IfSlotIsEmpty extends React.Component { + + shouldComponentUpdate(next) { + + // Prevent Slot from rerendering when only reduxState has changed and + // it does not result in a change. + const changes = getShallowChanges(this.props, next); + if (changes.length === 1 && changes[0] === 'reduxState') { + return this.isSlotEmpty(this.props) !== this.isSlotEmpty(next); + } + + // Prevent Slot from rerendering when no props has shallowly changed. + return changes.length !== 0; + } + + isSlotEmpty(props = this.props) { + const {slot, className: _a, reduxState, component: _b = 'div', children: _c, ...rest} = props; + return isSlotEmpty(slot, reduxState, rest); + } + + render() { + const {className, component: Component = 'div', children} = this.props; + return ( + + {this.isSlotEmpty() ? children : null} + + ); + } } IfSlotIsEmpty.propTypes = { diff --git a/client/coral-framework/components/IfSlotIsNotEmpty.js b/client/coral-framework/components/IfSlotIsNotEmpty.js index 3a41fffbd..e7a0e83ce 100644 --- a/client/coral-framework/components/IfSlotIsNotEmpty.js +++ b/client/coral-framework/components/IfSlotIsNotEmpty.js @@ -3,13 +3,36 @@ import {connect} from 'react-redux'; import {isSlotEmpty} from 'coral-framework/helpers/plugins'; import PropTypes from 'prop-types'; import omit from 'lodash/omit'; +import {getShallowChanges} from 'coral-framework/utils'; -function IfSlotIsNotEmpty({slot, className, reduxState, component: Component = 'div', children, ...rest}) { - return ( - - {!isSlotEmpty(slot, reduxState, rest) ? children : null} - - ); +class IfSlotIsNotEmpty extends React.Component { + + shouldComponentUpdate(next) { + + // Prevent Slot from rerendering when only reduxState has changed and + // it does not result in a change. + const changes = getShallowChanges(this.props, next); + if (changes.length === 1 && changes[0] === 'reduxState') { + return this.isSlotEmpty(this.props) !== this.isSlotEmpty(next); + } + + // Prevent Slot from rerendering when no props has shallowly changed. + return changes.length !== 0; + } + + isSlotEmpty(props = this.props) { + const {slot, className: _a, reduxState, component: _b = 'div', children: _c, ...rest} = props; + return isSlotEmpty(slot, reduxState, rest); + } + + render() { + const {className, component: Component = 'div', children} = this.props; + return ( + + {this.isSlotEmpty() ? null : children} + + ); + } } IfSlotIsNotEmpty.propTypes = { From f8031be86589e3cbdebfadb862eae77a8a1748b3 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 16 Aug 2017 23:41:14 +0700 Subject: [PATCH 08/40] callback props from withMutation now keep their identity --- .../src/components/Comment.js | 2 +- client/coral-framework/hocs/withMutation.js | 35 +++++++++++++++++-- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index b8063858d..6c231f4ae 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -72,7 +72,7 @@ const ActionButton = ({children}) => { ); }; -export default class Comment extends React.PureComponent { +export default class Comment extends React.Component { constructor(props) { super(props); diff --git a/client/coral-framework/hocs/withMutation.js b/client/coral-framework/hocs/withMutation.js index bb2321d11..7c3dcfa1a 100644 --- a/client/coral-framework/hocs/withMutation.js +++ b/client/coral-framework/hocs/withMutation.js @@ -9,6 +9,7 @@ import {getDefinitionName, getResponseErrors} from '../utils'; import PropTypes from 'prop-types'; import t from 'coral-framework/services/i18n'; import hoistStatics from 'recompose/hoistStatics'; +import union from 'lodash/union'; class ResponseErrors extends Error { constructor(errors) { @@ -47,7 +48,13 @@ export default (document, config = {}) => hoistStatics((WrappedComponent) => { // Lazily resolve fragments from graphRegistry to support circular dependencies. memoized = null; - wrappedProps = (data) => { + // Props as we would pass to the BaseComponent without optimizations. + dynamicProps = {}; + + // Props that are optimized by keeping the identity of function callbacks. + staticProps = {}; + + propsWrapper = (data) => { const name = getDefinitionName(document); const callbacks = getMutationOptions(name); const mutate = (base) => { @@ -133,12 +140,34 @@ export default (document, config = {}) => hoistStatics((WrappedComponent) => { throw error; }); }; - return config.props({...data, mutate}); + + // Save current props to `dynamicProps` + this.dynamicProps = config.props({...data, mutate}); + + // Sync props to `staticProps`. + // `staticProps` ultimately contains the same props as `dynamicProps` but all callbacks + // keep their identity. + union(Object.keys(this.dynamicProps), Object.keys(this.staticProps)).forEach((key) => { + if (!(key in this.dynamicProps)) { + delete this.staticProps[key]; + return; + } + if (typeof this.dynamicProps[key] !== 'function') { + this.staticProps[key] = this.dynamicProps[key]; + return; + } + + if (!(key in this.staticProps)) { + this.staticProps[key] = (...args) => this.dynamicProps[key](...args); + return; + } + }); + return this.staticProps; }; getWrapped = () => { if (!this.memoized) { - this.memoized = graphql(resolveFragments(document), {...config, props: this.wrappedProps})(WrappedComponent); + this.memoized = graphql(resolveFragments(document), {...config, props: this.propsWrapper})(WrappedComponent); } return this.memoized; }; From df141925fb755544f19b4999c0be3c73eac0eca2 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 17 Aug 2017 17:15:06 +0700 Subject: [PATCH 09/40] add hoistStatics to withReaction --- plugin-api/beta/client/hocs/withReaction.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugin-api/beta/client/hocs/withReaction.js b/plugin-api/beta/client/hocs/withReaction.js index 22c30d639..9ac49cf43 100644 --- a/plugin-api/beta/client/hocs/withReaction.js +++ b/plugin-api/beta/client/hocs/withReaction.js @@ -11,9 +11,10 @@ import {showSignInDialog} from 'coral-framework/actions/auth'; import {addNotification} from 'coral-framework/actions/notification'; import {capitalize} from 'coral-framework/helpers/strings'; import {getMyActionSummary, getTotalActionCount} from 'coral-framework/utils'; +import hoistStatics from 'recompose/hoistStatics'; import * as PropTypes from 'prop-types'; -export default (reaction) => (WrappedComponent) => { +export default (reaction) => hoistStatics((WrappedComponent) => { if (typeof reaction !== 'string') { console.error('Reaction must be a valid string'); return null; @@ -391,4 +392,4 @@ export default (reaction) => (WrappedComponent) => { WithReactions.displayName = `WithReactions(${getDisplayName(WrappedComponent)})`; return enhance(WithReactions); -}; +}); From 1ff6c5bdcc7aa95a637b31189f5a56bbb039cc66 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 17 Aug 2017 17:16:15 +0700 Subject: [PATCH 10/40] Use comment container --- .../src/components/AllCommentsPane.js | 2 +- .../src/components/Comment.js | 23 ++------- .../src/components/Stream.js | 2 +- .../src/containers/Comment.js | 47 +++++++++++++++++-- 4 files changed, 50 insertions(+), 24 deletions(-) diff --git a/client/coral-embed-stream/src/components/AllCommentsPane.js b/client/coral-embed-stream/src/components/AllCommentsPane.js index 6c5d6bb23..c141f0618 100644 --- a/client/coral-embed-stream/src/components/AllCommentsPane.js +++ b/client/coral-embed-stream/src/components/AllCommentsPane.js @@ -5,7 +5,7 @@ import IgnoredCommentTombstone from './IgnoredCommentTombstone'; import NewCount from './NewCount'; import {TransitionGroup} from 'react-transition-group'; import {forEachError} from 'coral-framework/utils'; -import Comment from '../components/Comment'; +import Comment from '../containers/Comment'; const hasComment = (nodes, id) => nodes.some((node) => node.id === id); diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index 6c231f4ae..39c3b5edf 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -24,6 +24,7 @@ import InactiveCommentLabel from './InactiveCommentLabel'; import {EditableCommentContent} from './EditableCommentContent'; import {getActionSummary, iPerformedThisAction, forEachError, isCommentActive} from 'coral-framework/utils'; import t from 'coral-framework/services/i18n'; +import CommentContainer from '../containers/Comment'; const isStaff = (tags) => !tags.every((t) => t.tag.name !== 'STAFF'); const hasTag = (tags, lookupTag) => !!tags.filter((t) => t.tag.name === lookupTag).length; @@ -86,7 +87,6 @@ export default class Comment extends React.Component { // Whether the comment should be editable (e.g. after a commenter clicking the 'Edit' button on their own comment) isEditing: false, replyBoxVisible: false, - animateEnter: false, loadingState: '', ...resetCursors({}, props), }; @@ -112,22 +112,6 @@ export default class Comment extends React.Component { } } - componentWillEnter(callback) { - callback(); - const userId = this.props.currentUser ? this.props.currentUser.id : null; - if (this.props.comment.id.indexOf('pending') >= 0) { - return; - } - if (userId && this.props.comment.user.id === userId) { - - // This comment was just added by currentUser. - if (Date.now() - Number(new Date(this.props.comment.created_at)) < 30 * 1000) { - return; - } - } - this.setState({animateEnter: true}); - } - static propTypes = { // id of currently opened ReplyBox. tracked in Stream.js @@ -315,6 +299,7 @@ export default class Comment extends React.Component { showSignInDialog, liveUpdates, commentIsIgnored, + animateEnter, commentClassNames = [] } = this.props; @@ -367,7 +352,7 @@ export default class Comment extends React.Component { styles[`rootLevel${depth}`], { ...conditionalClassNames, - [styles.enter]: this.state.animateEnter, + [styles.enter]: animateEnter, }, ); @@ -542,7 +527,7 @@ export default class Comment extends React.Component { {view.map((reply) => { return commentIsIgnored(reply) ? - : { + class WithAnimateEnter extends React.Component { + state = { + animateEnter: false, + }; + + componentWillEnter(callback) { + callback(); + const userId = this.props.currentUser ? this.props.currentUser.id : null; + if (this.props.comment.id.indexOf('pending') >= 0) { + return; + } + if (userId && this.props.comment.user.id === userId) { + + // This comment was just added by currentUser. + if (Date.now() - Number(new Date(this.props.comment.created_at)) < 30 * 1000) { + return; + } + } + this.setState({animateEnter: true}); + } + + render() { + return ; + } + } + return WithAnimateEnter; +}); + +const withCommentFragments = withFragments({ root: gql` fragment CoralEmbedStream_Comment_root on RootQuery { __typename @@ -57,4 +91,11 @@ export default withFragments({ ${getSlotFragmentSpreads(slots, 'comment')} } ` -})(Comment); +}); + +const enhance = compose( + withAnimateEnter, + withCommentFragments, +); + +export default enhance(Comment); From 7ea6d133c2e7a7693a2201a8d7b257ecb39eefad Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 17 Aug 2017 21:31:09 +0700 Subject: [PATCH 11/40] Filter queryData in withFragments and optimize rendering --- .../src/components/Comment.js | 18 +++- .../src/components/Stream.js | 29 ++++--- .../src/containers/Comment.js | 68 +++++++++------ .../src/containers/Stream.js | 33 ++----- .../src/containers/StreamTabPanel.js | 10 ++- client/coral-framework/components/Slot.js | 17 ++-- client/coral-framework/helpers/plugins.js | 39 +++++++-- client/coral-framework/hocs/withFragments.js | 85 ++++++++++++++++++- plugin-api/beta/client/hocs/withReaction.js | 11 ++- plugin-api/beta/client/hocs/withTags.js | 6 ++ .../client/containers/TabPane.js | 4 +- 11 files changed, 227 insertions(+), 93 deletions(-) diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index 39c3b5edf..cc1ee7bed 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -224,6 +224,10 @@ export default class Comment extends React.Component { return; } + commentPostedHandler = () => { + this.props.setActiveReplyBox(''); + } + // getVisibileReplies returns a list containing comments // which were authored by current user or comes before the `idCursor`. getVisibileReplies() { @@ -372,10 +376,13 @@ export default class Comment extends React.Component { // props that are passed down the slots. const slotProps = { data, + depth, + }; + + const queryData = { root, asset, comment, - depth, }; return ( @@ -389,6 +396,7 @@ export default class Comment extends React.Component { className={`${styles.commentAvatar} talk-stream-comment-avatar`} fill="commentAvatar" {...slotProps} + queryData={queryData} inline /> @@ -411,6 +419,7 @@ export default class Comment extends React.Component { className={styles.commentInfoBar} fill="commentInfoBar" {...slotProps} + queryData={queryData} /> { isActive && (currentUser && (comment.user.id === currentUser.id)) && @@ -457,6 +466,7 @@ export default class Comment extends React.Component { fill="commentContent" defaultComponent={CommentContent} {...slotProps} + queryData={queryData} /> } @@ -468,6 +478,7 @@ export default class Comment extends React.Component { {!disableReply && @@ -484,6 +495,7 @@ export default class Comment extends React.Component { fill="commentActions" wrapperComponent={ActionButton} {...slotProps} + queryData={queryData} inline /> @@ -509,9 +521,7 @@ export default class Comment extends React.Component { {activeReplyBox === comment.id ? { - setActiveReplyBox(''); - }} + commentPostedHandler={this.commentPostedHandler} charCountEnable={charCountEnable} maxCharCount={maxCharCount} setActiveReplyBox={setActiveReplyBox} diff --git a/client/coral-embed-stream/src/components/Stream.js b/client/coral-embed-stream/src/components/Stream.js index aa1221943..cf74813e9 100644 --- a/client/coral-embed-stream/src/components/Stream.js +++ b/client/coral-embed-stream/src/components/Stream.js @@ -40,6 +40,15 @@ class Stream extends React.Component { } } + commentIsIgnored = (comment) => { + const me = this.props.root.me; + return ( + me && + me.ignoredUsers && + me.ignoredUsers.find((u) => u.id === comment.user.id) + ); + }; + render() { const { data, @@ -48,7 +57,7 @@ class Stream extends React.Component { setActiveReplyBox, appendItemArray, commentClassNames, - root: {asset, asset: {comment, comments, totalCommentCount}, me}, + root: {asset, asset: {comment, comments, totalCommentCount}}, postComment, addNotification, editComment, @@ -89,16 +98,9 @@ class Stream extends React.Component { user.suspension.until && new Date(user.suspension.until) > new Date(); - const commentIsIgnored = (comment) => { - return ( - me && - me.ignoredUsers && - me.ignoredUsers.find((u) => u.id === comment.user.id) - ); - }; - const showCommentBox = loggedIn && ((!banned && !temporarilySuspended && !highlightedComment) || keepCommentBox); - const slotProps = {data, root, asset}; + const slotProps = {data}; + const slotQueryData = {root, asset}; if (!comment && !comments) { console.error('Talk: No comments came back from the graph given that query. Please, check the query params.'); @@ -160,6 +162,7 @@ class Stream extends React.Component { @@ -194,7 +197,7 @@ class Stream extends React.Component { deleteAction={deleteAction} showSignInDialog={showSignInDialog} key={highlightedComment.id} - commentIsIgnored={commentIsIgnored} + commentIsIgnored={this.commentIsIgnored} comment={highlightedComment} charCountEnable={asset.settings.charCountEnable} maxCharCount={asset.settings.charCount} @@ -209,6 +212,7 @@ class Stream extends React.Component { > @@ -219,6 +223,7 @@ class Stream extends React.Component { tabSlot={'streamTabs'} tabPaneSlot={'streamTabPanes'} slotProps={slotProps} + queryData={slotQueryData} appendTabs={ All Comments {totalCommentCount} @@ -245,7 +250,7 @@ class Stream extends React.Component { loadNewReplies={loadNewReplies} deleteAction={deleteAction} showSignInDialog={showSignInDialog} - commentIsIgnored={commentIsIgnored} + commentIsIgnored={this.commentIsIgnored} charCountEnable={asset.settings.charCountEnable} maxCharCount={asset.settings.charCount} editComment={editComment} diff --git a/client/coral-embed-stream/src/containers/Comment.js b/client/coral-embed-stream/src/containers/Comment.js index 40c2fa000..5f3ee0c9d 100644 --- a/client/coral-embed-stream/src/containers/Comment.js +++ b/client/coral-embed-stream/src/containers/Comment.js @@ -3,7 +3,9 @@ import React from 'react'; import Comment from '../components/Comment'; import {withFragments} from 'coral-framework/hocs'; import {getSlotFragmentSpreads} from 'coral-framework/utils'; +import {THREADING_LEVEL} from '../constants/stream'; import hoistStatics from 'recompose/hoistStatics'; +import {nest} from '../graphql/utils'; const slots = [ 'streamQuestionArea', @@ -48,6 +50,36 @@ const withAnimateEnter = hoistStatics((BaseComponent) => { return WithAnimateEnter; }); +const singleCommentFragment = gql` + fragment CoralEmbedStream_Comment_SingleComment on Comment { + id + body + created_at + status + replyCount + tags { + tag { + name + } + } + user { + id + username + } + action_summaries { + __typename + count + current_user { + id + } + } + editing { + edited + editableUntil + } + } +`; + const withCommentFragments = withFragments({ root: gql` fragment CoralEmbedStream_Comment_root on RootQuery { @@ -63,33 +95,21 @@ const withCommentFragments = withFragments({ `, comment: gql` fragment CoralEmbedStream_Comment_comment on Comment { - id - body - created_at - status - replyCount - tags { - tag { - name + ...CoralEmbedStream_Comment_SingleComment + ${nest(` + replies(limit: 3, excludeIgnored: $excludeIgnored) { + nodes { + ...CoralEmbedStream_Comment_SingleComment + ...nest + } + hasNextPage + startCursor + endCursor } - } - user { - id - username - } - action_summaries { - __typename - count - current_user { - id - } - } - editing { - edited - editableUntil - } + `, THREADING_LEVEL)} ${getSlotFragmentSpreads(slots, 'comment')} } + ${singleCommentFragment} ` }); diff --git a/client/coral-embed-stream/src/containers/Stream.js b/client/coral-embed-stream/src/containers/Stream.js index acc178261..f433bfb77 100644 --- a/client/coral-embed-stream/src/containers/Stream.js +++ b/client/coral-embed-stream/src/containers/Stream.js @@ -161,19 +161,11 @@ class StreamContainer extends React.Component { const commentFragment = gql` fragment CoralEmbedStream_Stream_comment on Comment { id + status + user { + id + } ...${getDefinitionName(Comment.fragments.comment)} - ${nest(` - replies(excludeIgnored: $excludeIgnored) { - nodes { - id - ...${getDefinitionName(Comment.fragments.comment)} - ...nest - } - hasNextPage - startCursor - endCursor - } - `, THREADING_LEVEL)} } ${Comment.fragments.comment} `; @@ -210,27 +202,14 @@ const LOAD_MORE_QUERY = gql` query CoralEmbedStream_LoadMoreComments($limit: Int = 5, $cursor: Date, $parent_id: ID, $asset_id: ID, $sort: SORT_ORDER, $excludeIgnored: Boolean) { comments(query: {limit: $limit, cursor: $cursor, parent_id: $parent_id, asset_id: $asset_id, sort: $sort, excludeIgnored: $excludeIgnored}) { nodes { - id - ...${getDefinitionName(Comment.fragments.comment)} - ${nest(` - replies(limit: 3, excludeIgnored: $excludeIgnored) { - nodes { - id - ...${getDefinitionName(Comment.fragments.comment)} - ...nest - } - hasNextPage - startCursor - endCursor - } - `, THREADING_LEVEL)} + ...CoralEmbedStream_Stream_comment } hasNextPage startCursor endCursor } } - ${Comment.fragments.comment} + ${commentFragment} `; const slots = [ diff --git a/client/coral-embed-stream/src/containers/StreamTabPanel.js b/client/coral-embed-stream/src/containers/StreamTabPanel.js index 4a313d2b4..7b91d58ee 100644 --- a/client/coral-embed-stream/src/containers/StreamTabPanel.js +++ b/client/coral-embed-stream/src/containers/StreamTabPanel.js @@ -2,7 +2,7 @@ import React from 'react'; import StreamTabPanel from '../components/StreamTabPanel'; import {connect} from 'react-redux'; import omit from 'lodash/omit'; -import {getSlotComponents} from 'coral-framework/helpers/plugins'; +import {getSlotComponents, getSlotComponentProps} from 'coral-framework/helpers/plugins'; import {Tab, TabPane} from 'coral-ui'; import {getShallowChanges} from 'coral-framework/utils'; import isEqual from 'lodash/isEqual'; @@ -43,14 +43,14 @@ class StreamTabPanelContainer extends React.Component { } getSlotComponents(slot, props = this.props) { - return getSlotComponents(slot, props.reduxState, props.slotProps); + return getSlotComponents(slot, props.reduxState, props.slotProps, props.queryData); } getPluginTabElements(props = this.props) { return this.getSlotComponents(props.tabSlot).map((PluginComponent) => ( @@ -61,7 +61,7 @@ class StreamTabPanelContainer extends React.Component { return this.getSlotComponents(props.tabPaneSlot).map((PluginComponent) => ( )); @@ -95,6 +95,8 @@ StreamTabPanelContainer.propTypes = { fallbackTab: PropTypes.string.isRequired, tabSlot: PropTypes.string.isRequired, tabPaneSlot: PropTypes.string.isRequired, + slotProps: PropTypes.object.isRequired, + queryData: PropTypes.object, className: PropTypes.string, sub: PropTypes.bool, }; diff --git a/client/coral-framework/components/Slot.js b/client/coral-framework/components/Slot.js index 0b0f4d325..838382978 100644 --- a/client/coral-framework/components/Slot.js +++ b/client/coral-framework/components/Slot.js @@ -2,11 +2,13 @@ import React from 'react'; import cn from 'classnames'; import styles from './Slot.css'; import {connect} from 'react-redux'; -import {getSlotElements} from 'coral-framework/helpers/plugins'; +import {getSlotElements, getSlotComponentProps} from 'coral-framework/helpers/plugins'; import omit from 'lodash/omit'; import isEqual from 'lodash/isEqual'; import {getShallowChanges} from 'coral-framework/utils'; +const emptyConfig = {}; + class Slot extends React.Component { shouldComponentUpdate(next) { @@ -23,20 +25,20 @@ class Slot extends React.Component { return changes.length !== 0; } - getSlotProps({fill: _a, inline: _b, className: _c, reduxState: _d, defaultComponent_: _e, ...rest} = this.props) { + getSlotProps({fill: _a, inline: _b, className: _c, reduxState: _d, defaultComponent_: _e, queryData: _f, ...rest} = this.props) { return rest; } getChildren(props = this.props) { - return getSlotElements(props.fill, props.reduxState, this.getSlotProps(props)); + return getSlotElements(props.fill, props.reduxState, this.getSlotProps(props), props.queryData); } render() { - const {inline = false, className, reduxState, defaultComponent: DefaultComponent} = this.props; + const {inline = false, className, reduxState, defaultComponent: DefaultComponent, queryData} = this.props; let children = this.getChildren(); - const pluginConfig = reduxState.config.pluginConfig || {}; + const pluginConfig = reduxState.config.pluginConfig || emptyConfig; if (children.length === 0 && DefaultComponent) { - children = ; + children = ; } return ( @@ -48,7 +50,8 @@ class Slot extends React.Component { } Slot.propTypes = { - fill: React.PropTypes.string.isRequired + fill: React.PropTypes.string.isRequired, + queryData: React.PropTypes.object, }; const mapStateToProps = (state) => ({ diff --git a/client/coral-framework/helpers/plugins.js b/client/coral-framework/helpers/plugins.js index 7bebaa1f6..7f6e5f1c4 100644 --- a/client/coral-framework/helpers/plugins.js +++ b/client/coral-framework/helpers/plugins.js @@ -11,8 +11,11 @@ import camelize from './camelize'; import plugins from 'pluginsConfig'; import uuid from 'uuid/v4'; -export function getSlotComponents(slot, reduxState, props = {}) { - const pluginConfig = reduxState.config.plugin_config || {}; +// This is returned for pluginConfig when it is empty. +const emptyConfig = {}; + +export function getSlotComponents(slot, reduxState, props = {}, queryData = {}) { + const pluginConfig = reduxState.config.plugin_config || emptyConfig; return flatten(plugins // Filter out components that have slots and have been disabled in `plugin_config` @@ -25,7 +28,7 @@ export function getSlotComponents(slot, reduxState, props = {}) { if(!component.isExcluded) { return true; } - let resolvedProps = {...props, config: pluginConfig}; + let resolvedProps = getSlotComponentProps(component, reduxState, props, queryData); if (component.mapStateToProps) { resolvedProps = {...resolvedProps, ...component.mapStateToProps(reduxState)}; } @@ -33,17 +36,35 @@ export function getSlotComponents(slot, reduxState, props = {}) { }); } -export function isSlotEmpty(slot, reduxState, props) { - return getSlotComponents(slot, reduxState, props).length === 0; +export function isSlotEmpty(slot, reduxState, props = {}, queryData = {}) { + return getSlotComponents(slot, reduxState, props, queryData).length === 0; +} + +/** + * getSlotComponentProps calculate the props we would pass to the slot component. + * query datas are only passed to the component if it is defined in `component.fragments`. + */ +export function getSlotComponentProps(component, reduxState, props, queryData) { + const pluginConfig = reduxState.config.plugin_config || emptyConfig; + return { + ...props, + config: pluginConfig, + ...( + component.fragments + ? pick(queryData, Object.keys(component.fragments)) + : queryData // TODO: should be {} + ) + }; } /** * Returns React Elements for given slot. */ -export function getSlotElements(slot, reduxState, props = {}) { - const pluginConfig = reduxState.config.plugin_config || {}; - return getSlotComponents(slot, reduxState, props) - .map((component, i) => React.createElement(component, {key: i, ...props, config: pluginConfig})); +export function getSlotElements(slot, reduxState, props = {}, queryData = {}) { + return getSlotComponents(slot, reduxState, props, queryData) + .map((component, i) => { + return React.createElement(component, {key: i, ...getSlotComponentProps(component, reduxState, props, queryData)}); + }); } export function getSlotFragments(slot, part) { diff --git a/client/coral-framework/hocs/withFragments.js b/client/coral-framework/hocs/withFragments.js index c422f8705..aef7147d8 100644 --- a/client/coral-framework/hocs/withFragments.js +++ b/client/coral-framework/hocs/withFragments.js @@ -1,17 +1,98 @@ -// TODO: revisit `filtering` after https://github.com/apollographql/graphql-anywhere/issues/38. - import React from 'react'; +import graphql from 'graphql-anywhere'; import {resolveFragments} from 'coral-framework/services/graphqlRegistry'; import mapValues from 'lodash/mapValues'; import hoistStatics from 'recompose/hoistStatics'; +import {getShallowChanges} from 'coral-framework/utils'; + +// TODO: Should not depend on `props.data` +// Currently necessary because of this https://github.com/apollographql/graphql-anywhere/issues/38 +function filter(doc, data, variables) { + const resolver = ( + fieldName, + root, + args, + context, + info, + ) => { + return root[info.resultKey]; + }; + + return graphql(resolver, doc, data, null, variables); +} + +// filterProps returns only the property as defined in the fragments. +// TODO: Should not depend on `props.data` +function filterProps(props, fragments) { + const filtered = {}; + Object.keys(fragments).forEach((key) => { + if (!(key in props)) { + return; + } + filtered[key] = filter(fragments[key], props[key], props.data.variables); + }); + return filtered; +} + +// hasEqualLeaves compares two different apollo query result for equality. +function hasEqualLeaves(a, b, path = '') { + for (const key in a) { + if (typeof a[key] === 'object') { + if (Array.isArray(a[key])) { + if (a[key].length !== b[key].length) { + return false; + } + } + if (!hasEqualLeaves(a[key], b[key], `${path}.${key}`)) { + return false; + } + continue; + } + if (a[key] !== b[key]) { + return false; + } + } + return true; +} export default (fragments) => hoistStatics((BaseComponent) => { class WithFragments extends React.Component { fragments = mapValues(fragments, (val) => resolveFragments(val)); + fragmentKeys = Object.keys(fragments).sort(); + + // Cache variables between lifecycles to speed up render. + filteredProps = filterProps(this.props, this.fragments) + queryDataHasChanged = false; + lastFilteredProps = null; + shallowChanges = null; + + componentWillReceiveProps(next) { + this.shallowChanges = getShallowChanges(this.props, next); + this.queryDataHasChanged = this.fragmentKeys.some((key) => this.shallowChanges.indexOf(key) >= 0); + + if (this.queryDataHasChanged) { + + // If query data has changed, we compute the next filtered props. + this.lastFilteredProps = this.filteredProps; + this.filteredProps = filterProps(next, this.fragments); + } + } + + shouldComponentUpdate(next) { + + // If only query data was changed. + if (this.queryDataHasChanged && this.shallowChanges.every((key) => this.fragmentKeys.indexOf(key) >= 0)) { + return !hasEqualLeaves(this.lastFilteredProps, this.filteredProps); + } + + return this.shallowChanges.length !== 0; + } render() { + const queryProps = this.filteredProps; return ; } } diff --git a/plugin-api/beta/client/hocs/withReaction.js b/plugin-api/beta/client/hocs/withReaction.js index 9ac49cf43..0d118a12f 100644 --- a/plugin-api/beta/client/hocs/withReaction.js +++ b/plugin-api/beta/client/hocs/withReaction.js @@ -176,7 +176,7 @@ export default (reaction) => hoistStatics((WrappedComponent) => { createdSubscription = context.client.subscribe({ query: REACTION_CREATED_SUBSCRIPTION, variables: { - assetId: this.props.root.asset.id, + assetId: this.props.asset.id, }, }).subscribe({ next: this.onReactionCreated, @@ -186,7 +186,7 @@ export default (reaction) => hoistStatics((WrappedComponent) => { deletedSubscription = context.client.subscribe({ query: REACTION_DELETED_SUBSCRIPTION, variables: { - assetId: this.props.root.asset.id, + assetId: this.props.asset.id, }, }).subscribe({ next: this.onReactionDeleted, @@ -372,9 +372,16 @@ export default (reaction) => hoistStatics((WrappedComponent) => { const enhance = compose( withFragments({ + asset: gql` + fragment ${Reaction}Button_asset on Asset { + id + } + `, comment: gql` fragment ${Reaction}Button_comment on Comment { + id action_summaries { + __typename ... on ${Reaction}ActionSummary { count current_user { diff --git a/plugin-api/beta/client/hocs/withTags.js b/plugin-api/beta/client/hocs/withTags.js index 428c3f43f..b1d565557 100644 --- a/plugin-api/beta/client/hocs/withTags.js +++ b/plugin-api/beta/client/hocs/withTags.js @@ -93,8 +93,14 @@ export default (tag) => hoistStatics((WrappedComponent) => { const enhance = compose( withFragments({ + asset: gql` + fragment ${Tag}Button_asset on Asset { + id + } + `, comment: gql` fragment ${Tag}Button_comment on Comment { + id tags { tag { name diff --git a/plugins/talk-plugin-featured-comments/client/containers/TabPane.js b/plugins/talk-plugin-featured-comments/client/containers/TabPane.js index 2195b0107..63ce30d01 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/TabPane.js +++ b/plugins/talk-plugin-featured-comments/client/containers/TabPane.js @@ -16,8 +16,8 @@ class TabPaneContainer extends React.Component { query: LOAD_MORE_QUERY, variables: { limit: 5, - cursor: this.props.root.asset.featuredComments.endCursor, - asset_id: this.props.root.asset.id, + cursor: this.props.asset.featuredComments.endCursor, + asset_id: this.props.asset.id, sort: 'REVERSE_CHRONOLOGICAL', excludeIgnored: this.props.data.variables.excludeIgnored, }, From d228d49c7d0388bfbfa7800d4bd7826b44b2cdf3 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 17 Aug 2017 22:44:13 +0700 Subject: [PATCH 12/40] Optimize rendering when `activeReplyBox` changes --- .../src/components/Comment.js | 31 ++++++++++++++++++- .../coral-embed-stream/src/graphql/utils.js | 6 +++- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index cc1ee7bed..d34de96ac 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -16,13 +16,14 @@ import mapValues from 'lodash/mapValues'; import LoadMore from './LoadMore'; import {getEditableUntilDate} from './util'; +import {findCommentWithId} from '../graphql/utils'; import {TopRightMenu} from './TopRightMenu'; import CommentContent from './CommentContent'; import Slot from 'coral-framework/components/Slot'; import IgnoredCommentTombstone from './IgnoredCommentTombstone'; import InactiveCommentLabel from './InactiveCommentLabel'; import {EditableCommentContent} from './EditableCommentContent'; -import {getActionSummary, iPerformedThisAction, forEachError, isCommentActive} from 'coral-framework/utils'; +import {getActionSummary, iPerformedThisAction, forEachError, isCommentActive, getShallowChanges} from 'coral-framework/utils'; import t from 'coral-framework/services/i18n'; import CommentContainer from '../containers/Comment'; @@ -73,6 +74,17 @@ const ActionButton = ({children}) => { ); }; +// Determine whether the comment with id is in the part of the comments tree. +function containsCommentId(props, id) { + if (props.comment.id === id) { + return true; + } + if (props.comment.replies) { + return findCommentWithId(props.comment.replies.nodes, id); + } + return false; +} + export default class Comment extends React.Component { constructor(props) { @@ -112,6 +124,23 @@ export default class Comment extends React.Component { } } + shouldComponentUpdate(next) { + + // Specifically handle `activeReplyBox` if it is the only change. + const changes = getShallowChanges(this.props, next); + if (changes.length === 1 && changes[0] === 'activeReplyBox') { + if ( + !containsCommentId(next, this.props.activeReplyBox) && + !containsCommentId(next, next.activeReplyBox) + ) { + return false; + } + } + + // Prevent Slot from rerendering when no props has shallowly changed. + return changes.length !== 0; + } + static propTypes = { // id of currently opened ReplyBox. tracked in Stream.js diff --git a/client/coral-embed-stream/src/graphql/utils.js b/client/coral-embed-stream/src/graphql/utils.js index ac6298616..11d3d411b 100644 --- a/client/coral-embed-stream/src/graphql/utils.js +++ b/client/coral-embed-stream/src/graphql/utils.js @@ -117,7 +117,7 @@ export function getTopLevelParent(comment) { return comment; } -function findComment(nodes, callback) { +export function findComment(nodes, callback) { for (let i = 0; i < nodes.length; i++) { const node = nodes[i]; if (callback(node)) { @@ -133,6 +133,10 @@ function findComment(nodes, callback) { return false; } +export function findCommentWithId(nodes, id) { + return findComment(nodes, (node) => node.id === id); +} + export function findCommentInEmbedQuery(root, callbackOrId) { let callback = callbackOrId; if (typeof callbackOrId === 'string') { From 25c11ad690b0da8bd6fbe9e754dbf911c788780d Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 17 Aug 2017 23:22:30 +0700 Subject: [PATCH 13/40] Keep old query data when not changed in withFragments --- client/coral-framework/hocs/withFragments.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/client/coral-framework/hocs/withFragments.js b/client/coral-framework/hocs/withFragments.js index aef7147d8..76a6f7035 100644 --- a/client/coral-framework/hocs/withFragments.js +++ b/client/coral-framework/hocs/withFragments.js @@ -63,18 +63,19 @@ export default (fragments) => hoistStatics((BaseComponent) => { // Cache variables between lifecycles to speed up render. filteredProps = filterProps(this.props, this.fragments) queryDataHasChanged = false; - lastFilteredProps = null; shallowChanges = null; componentWillReceiveProps(next) { this.shallowChanges = getShallowChanges(this.props, next); - this.queryDataHasChanged = this.fragmentKeys.some((key) => this.shallowChanges.indexOf(key) >= 0); - if (this.queryDataHasChanged) { + if (this.fragmentKeys.some((key) => this.shallowChanges.indexOf(key) >= 0)) { + const nextFilteredProps = filterProps(next, this.fragments); + this.queryDataHasChanged = !hasEqualLeaves(this.filteredProps, nextFilteredProps); + if (this.queryDataHasChanged) { - // If query data has changed, we compute the next filtered props. - this.lastFilteredProps = this.filteredProps; - this.filteredProps = filterProps(next, this.fragments); + // Only changed props when query data has changed. + this.filteredProps = filterProps(next, this.fragments); + } } } @@ -82,7 +83,7 @@ export default (fragments) => hoistStatics((BaseComponent) => { // If only query data was changed. if (this.queryDataHasChanged && this.shallowChanges.every((key) => this.fragmentKeys.indexOf(key) >= 0)) { - return !hasEqualLeaves(this.lastFilteredProps, this.filteredProps); + return this.queryDataHasChanged; } return this.shallowChanges.length !== 0; From af4b01c2aee7a91a0b250c5902b78d99214fc486 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 17 Aug 2017 23:23:18 +0700 Subject: [PATCH 14/40] Correctly handly activeReplyBox optimization --- client/coral-embed-stream/src/components/Comment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index d34de96ac..dcc52d85a 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -130,7 +130,7 @@ export default class Comment extends React.Component { const changes = getShallowChanges(this.props, next); if (changes.length === 1 && changes[0] === 'activeReplyBox') { if ( - !containsCommentId(next, this.props.activeReplyBox) && + !containsCommentId(this.props, this.props.activeReplyBox) && !containsCommentId(next, next.activeReplyBox) ) { return false; From 0808f97fb59c5c30537b98494c07b60eb61dcaeb Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 16:40:42 +0700 Subject: [PATCH 15/40] Consider state when optimizing comment --- client/coral-embed-stream/src/components/Comment.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/coral-embed-stream/src/components/Comment.js b/client/coral-embed-stream/src/components/Comment.js index dcc52d85a..5a2572574 100644 --- a/client/coral-embed-stream/src/components/Comment.js +++ b/client/coral-embed-stream/src/components/Comment.js @@ -124,14 +124,14 @@ export default class Comment extends React.Component { } } - shouldComponentUpdate(next) { + shouldComponentUpdate(nextProps, nextState) { // Specifically handle `activeReplyBox` if it is the only change. - const changes = getShallowChanges(this.props, next); + const changes = [...getShallowChanges(this.props, nextProps), ...getShallowChanges(this.state, nextState)]; if (changes.length === 1 && changes[0] === 'activeReplyBox') { if ( !containsCommentId(this.props, this.props.activeReplyBox) && - !containsCommentId(next, next.activeReplyBox) + !containsCommentId(nextProps, nextProps.activeReplyBox) ) { return false; } From fabc8b8c46ce8ac66f27e02890a8913edaf71678 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 17:49:57 +0700 Subject: [PATCH 16/40] Check when one of the leaves are empty --- client/coral-framework/hocs/withFragments.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/client/coral-framework/hocs/withFragments.js b/client/coral-framework/hocs/withFragments.js index 76a6f7035..33c39abd0 100644 --- a/client/coral-framework/hocs/withFragments.js +++ b/client/coral-framework/hocs/withFragments.js @@ -4,6 +4,7 @@ import {resolveFragments} from 'coral-framework/services/graphqlRegistry'; import mapValues from 'lodash/mapValues'; import hoistStatics from 'recompose/hoistStatics'; import {getShallowChanges} from 'coral-framework/utils'; +import union from 'lodash/union'; // TODO: Should not depend on `props.data` // Currently necessary because of this https://github.com/apollographql/graphql-anywhere/issues/38 @@ -36,8 +37,11 @@ function filterProps(props, fragments) { // hasEqualLeaves compares two different apollo query result for equality. function hasEqualLeaves(a, b, path = '') { - for (const key in a) { - if (typeof a[key] === 'object') { + for (const key of union(Object.keys(a), Object.keys(b))) { + if (!(key in a) || !(key in b)) { + return false; + } + if (typeof a[key] === 'object' && a[key] && b[key]) { if (Array.isArray(a[key])) { if (a[key].length !== b[key].length) { return false; From 6a5f6e3ce1745ca7ca8b793ba940b3fbb7fd8979 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 17:57:32 +0700 Subject: [PATCH 17/40] Adapt loading detection --- client/coral-admin/src/containers/UserDetail.js | 2 +- client/coral-embed-stream/src/containers/Stream.js | 6 +++++- client/coral-settings/containers/ProfileContainer.js | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/client/coral-admin/src/containers/UserDetail.js b/client/coral-admin/src/containers/UserDetail.js index 2ae6e4c9c..88c43ce5f 100644 --- a/client/coral-admin/src/containers/UserDetail.js +++ b/client/coral-admin/src/containers/UserDetail.js @@ -108,7 +108,7 @@ class UserDetailContainer extends React.Component { return null; } - const loading = [1, 2, 4].indexOf(this.props.data.networkStatus) >= 0; + const loading = this.props.data.loading; return ; } return = 0; + const loading = this.props.data.loading; if (!auth.loggedIn) { return ; From 8eaac3eaa20aadab7ccc5fd1535777453f7723fe Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 19:50:56 +0700 Subject: [PATCH 18/40] Tiny refactor.. --- .../containers/ProfileContainer.js | 5 +- client/talk-plugin-history/Comment.js | 83 ++++++++++--------- client/talk-plugin-history/CommentHistory.js | 6 +- 3 files changed, 49 insertions(+), 45 deletions(-) diff --git a/client/coral-settings/containers/ProfileContainer.js b/client/coral-settings/containers/ProfileContainer.js index 3423cb6e2..6ffad3c87 100644 --- a/client/coral-settings/containers/ProfileContainer.js +++ b/client/coral-settings/containers/ProfileContainer.js @@ -51,7 +51,7 @@ class ProfileContainer extends Component { }; render() { - const {auth, auth: {user}, asset, showSignInDialog, stopIgnoringUser} = this.props; + const {auth, auth: {user}, showSignInDialog, stopIgnoringUser, root, data} = this.props; const {me} = this.props.root; const loading = this.props.data.loading; @@ -87,7 +87,7 @@ class ProfileContainer extends Component {

{t('framework.my_comments')}

{me.comments.nodes.length - ? + ? :

{t('user_no_comment')}

} ); @@ -138,7 +138,6 @@ const withProfileQuery = withQuery( `); const mapStateToProps = (state) => ({ - asset: state.asset, auth: state.auth }); diff --git a/client/talk-plugin-history/Comment.js b/client/talk-plugin-history/Comment.js index f90362e2d..d1640cb5e 100644 --- a/client/talk-plugin-history/Comment.js +++ b/client/talk-plugin-history/Comment.js @@ -7,54 +7,57 @@ import CommentContent from '../coral-embed-stream/src/components/CommentContent' import t from 'coral-framework/services/i18n'; -const Comment = (props) => { - return ( - - - ); -}; + ); + } +} Comment.propTypes = { comment: PropTypes.shape({ id: PropTypes.string, body: PropTypes.string }).isRequired, - asset: PropTypes.shape({ - url: PropTypes.string, - title: PropTypes.string - }).isRequired }; export default Comment; diff --git a/client/talk-plugin-history/CommentHistory.js b/client/talk-plugin-history/CommentHistory.js index d584d6a0a..43ae93f1d 100644 --- a/client/talk-plugin-history/CommentHistory.js +++ b/client/talk-plugin-history/CommentHistory.js @@ -22,16 +22,18 @@ class CommentHistory extends React.Component { } render() { - const {link, comments} = this.props; + const {link, comments, data, root} = this.props; return (
{comments.nodes.map((comment, i) => { return ; + />; })}
{comments.hasNextPage && From dc49d23dc8d12776d1a51f98b6568d297be1a60f Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 19:55:51 +0700 Subject: [PATCH 19/40] Define fragments for PermalinkButton --- .../client/containers/PermalinkButton.js | 16 ++++++++++++++++ plugins/talk-plugin-permalink/client/index.js | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 plugins/talk-plugin-permalink/client/containers/PermalinkButton.js diff --git a/plugins/talk-plugin-permalink/client/containers/PermalinkButton.js b/plugins/talk-plugin-permalink/client/containers/PermalinkButton.js new file mode 100644 index 000000000..5e1d11110 --- /dev/null +++ b/plugins/talk-plugin-permalink/client/containers/PermalinkButton.js @@ -0,0 +1,16 @@ +import {gql} from 'react-apollo'; +import PermalinkButton from '../components/PermalinkButton'; +import {withFragments} from 'plugin-api/beta/client/hocs'; + +export default withFragments({ + asset: gql` + fragment TalkPermalink_Button_asset on Asset { + url + } + `, + comment: gql` + fragment TalkPermalink_Button_comment on Comment { + id + } + ` +})(PermalinkButton); diff --git a/plugins/talk-plugin-permalink/client/index.js b/plugins/talk-plugin-permalink/client/index.js index d413da870..c28c4ae71 100644 --- a/plugins/talk-plugin-permalink/client/index.js +++ b/plugins/talk-plugin-permalink/client/index.js @@ -1,4 +1,4 @@ -import PermalinkButton from './components/PermalinkButton'; +import PermalinkButton from './containers/PermalinkButton'; export default { slots: { From 1dcdccecbc1f3699e055db6290b3d90db2caf0db Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Fri, 18 Aug 2017 10:19:10 -0300 Subject: [PATCH 20/40] UserDetail UI redone and Flagging Reliability added --- .../coral-admin/src/components/UserDetail.css | 100 ++++++++++++++---- .../coral-admin/src/components/UserDetail.js | 65 +++++++----- client/coral-ui/components/Icon.css | 2 + 3 files changed, 118 insertions(+), 49 deletions(-) diff --git a/client/coral-admin/src/components/UserDetail.css b/client/coral-admin/src/components/UserDetail.css index 20e090f47..5cbe0fe32 100644 --- a/client/coral-admin/src/components/UserDetail.css +++ b/client/coral-admin/src/components/UserDetail.css @@ -1,6 +1,78 @@ .copyButton { - float: right; - top: -10px; + background-color: white; + border: solid 1px; + padding: 2px 6px; + height: auto; + line-height: initial; + min-width: auto; + letter-spacing: normal; + font-size: 0.9em; + margin-left: 10px; +} + +.userDetailList { + list-style: none; + padding: 0; + margin: 0; +} + +.userDetailItem { + margin: 0 5px; + font-weight: 500; +} + +.stats { + display: flex; + list-style: none; + padding: 0; + margin: 0; + text-align: center; + margin: 15px 0 5px; + color: #595959; +} + +.stat { + margin-right: 20px; +} + +.stat:last-child { + margin-right: 0px; +} + +.statItem, .statReportResult { + padding: 3px 5px; + background-color: #D8D8D8; + border-radius: 3px; + font-weight: 500; + display: block; + font-size: 0.9em; + line-height: normal; + letter-spacing: 0.4px; + min-width: 60px; +} + +.statResult { + font-size: 1.5em; + padding: 5px 0; + display: inline-block; +} + +.statReportResult { + color: white; + margin: 5px 0; + font-weight: 400; +} + +.statReportResult.reliable { + background-color: #749C48; +} + +.statReportResult.neutral { + background-color: #616161; +} + +.statReportResult.unreliable { + background-color: #F44336; } .memberSince { @@ -8,27 +80,9 @@ } .small { - color: #aaa; -} - -.stats { - display: flex; - - .stat { - margin: 0 4px 10px 0px; - } - - .stat:last-child { - margin-right: 0; - } - - p { - margin: 0; - } - - .stat p:first-child { - font-weight: bold; - } + color: #888888; + font-size: 0.9em; + letter-spacing: 0.4px; } .profileEmail { diff --git a/client/coral-admin/src/components/UserDetail.js b/client/coral-admin/src/components/UserDetail.js index 34773ddef..64018cde8 100644 --- a/client/coral-admin/src/components/UserDetail.js +++ b/client/coral-admin/src/components/UserDetail.js @@ -1,12 +1,14 @@ -import React, {PropTypes} from 'react'; +import React from 'react'; +import PropTypes from 'prop-types'; import Comment from './UserDetailComment'; import styles from './UserDetail.css'; -import {Button, Drawer, Spinner} from 'coral-ui'; +import {Icon, Button, Drawer, Spinner} from 'coral-ui'; import {Slot} from 'coral-framework/components'; import ButtonCopyToClipboard from './ButtonCopyToClipboard'; import {actionsMap} from '../utils/moderationQueueActionsMap'; import ClickOutside from 'coral-framework/components/ClickOutside'; import LoadMore from '../components/LoadMore'; +import cn from 'classnames'; export default class UserDetail extends React.Component { @@ -74,13 +76,6 @@ export default class UserDetail extends React.Component { loadMore, } = this.props; - const localProfile = user.profiles.find((p) => p.provider === 'local'); - - let profile; - if (localProfile) { - profile = localProfile.id; - } - let rejectedPercent = (rejectedComments / totalComments) * 100; if (rejectedPercent === Infinity || isNaN(rejectedPercent)) { @@ -94,8 +89,40 @@ export default class UserDetail extends React.Component {

{user.username}

- {profile && this.profile = ref} value={profile} />} - +
    +
  • + + Member Since: + {new Date(user.created_at).toLocaleString()} +
    • + + {user.profiles.map(({id}) => +
  • + + Email: + {id} +
    • + )} +
+ +
    +
  • + Total Comments + {totalComments} +
    • +
  • + Reject Rate + {`${(rejectedPercent).toFixed(1)}%`} +
    • +
  • + Reports + Reliable +
    • +
+ +

+ Data represents the last six months of activity +

-

Member since {new Date(user.created_at).toLocaleString()}

+
-

- Account summary -
Data represents the last six months of activity -

-
-
-

Total Comments

-

{totalComments}

-
-
-

Reject Rate

-

{`${(rejectedPercent).toFixed(1)}%`}

-
-
{ selectedCommentIds.length === 0 ? ( diff --git a/client/coral-ui/components/Icon.css b/client/coral-ui/components/Icon.css index 16fe6d235..1a118257a 100644 --- a/client/coral-ui/components/Icon.css +++ b/client/coral-ui/components/Icon.css @@ -1,3 +1,5 @@ .root { + vertical-align: middle; + font-size: inherit; } From 9f62754fd77f8fcdcdc99d4cc97b9add8011850f Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 20:33:37 +0700 Subject: [PATCH 21/40] Show warnings when slot components uses query data without fragments --- client/coral-framework/helpers/plugins.js | 32 ++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/client/coral-framework/helpers/plugins.js b/client/coral-framework/helpers/plugins.js index 7f6e5f1c4..8fc3310e6 100644 --- a/client/coral-framework/helpers/plugins.js +++ b/client/coral-framework/helpers/plugins.js @@ -5,8 +5,10 @@ import merge from 'lodash/merge'; import flattenDeep from 'lodash/flattenDeep'; import isEmpty from 'lodash/isEmpty'; import flatten from 'lodash/flatten'; +import mapValues from 'lodash/mapValues'; import {loadTranslations} from 'coral-framework/services/i18n'; import {injectReducers} from 'coral-framework/services/store'; +import {getDisplayName} from 'coral-framework/helpers/hoc'; import camelize from './camelize'; import plugins from 'pluginsConfig'; import uuid from 'uuid/v4'; @@ -40,6 +42,34 @@ export function isSlotEmpty(slot, reduxState, props = {}, queryData = {}) { return getSlotComponents(slot, reduxState, props, queryData).length === 0; } +// Memoize the warnings so we only show them once. +const memoizedWarnings = []; + +function withWarnings(component, queryData) { + if (process.env.NODE_ENV !== 'production') { + + // Show warnings when accessing queryData only when not in production. + return mapValues(queryData, (value, key) => { + return new Proxy(queryData[key], { + get(target, name) { + + // Only care about the components defined in the plugins. + if (component.talkPluginName) { + const warning = `'${getDisplayName(component)}' of '${component.talkPluginName}' accessed '${key}.${name}' but did not define fragments using the withFragment HOC`; + if (memoizedWarnings.indexOf(warning) === -1) { + console.warn(warning); + memoizedWarnings.push(warning); + } + } + return queryData[key][name]; + } + }); + }); + } + + return queryData; +} + /** * getSlotComponentProps calculate the props we would pass to the slot component. * query datas are only passed to the component if it is defined in `component.fragments`. @@ -52,7 +82,7 @@ export function getSlotComponentProps(component, reduxState, props, queryData) { ...( component.fragments ? pick(queryData, Object.keys(component.fragments)) - : queryData // TODO: should be {} + : withWarnings(component, queryData) ) }; } From 00f365ab38d7d8dfd1c689add5e3d7bf552c0623 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 20:40:51 +0700 Subject: [PATCH 22/40] Add missing fragments --- .../client/containers/Tag.js | 15 +++++++++++++++ .../talk-plugin-featured-comments/client/index.js | 2 +- .../client/containers/OffTopicTag.js | 15 +++++++++++++++ plugins/talk-plugin-offtopic/client/index.js | 2 +- 4 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 plugins/talk-plugin-featured-comments/client/containers/Tag.js create mode 100644 plugins/talk-plugin-offtopic/client/containers/OffTopicTag.js diff --git a/plugins/talk-plugin-featured-comments/client/containers/Tag.js b/plugins/talk-plugin-featured-comments/client/containers/Tag.js new file mode 100644 index 000000000..60d9e1e2b --- /dev/null +++ b/plugins/talk-plugin-featured-comments/client/containers/Tag.js @@ -0,0 +1,15 @@ +import {gql} from 'react-apollo'; +import Tag from '../components/Tag'; +import {withFragments} from 'plugin-api/beta/client/hocs'; + +export default withFragments({ + comment: gql` + fragment TalkFeaturedComments_Tag_comment on Comment { + tags { + tag { + name + } + } + } + ` +})(Tag); diff --git a/plugins/talk-plugin-featured-comments/client/index.js b/plugins/talk-plugin-featured-comments/client/index.js index ad10f87ca..79c3d874f 100644 --- a/plugins/talk-plugin-featured-comments/client/index.js +++ b/plugins/talk-plugin-featured-comments/client/index.js @@ -1,5 +1,5 @@ import Tab from './containers/Tab'; -import Tag from './components/Tag'; +import Tag from './containers/Tag'; import Button from './components/Button'; import TabPane from './containers/TabPane'; import translations from './translations.yml'; diff --git a/plugins/talk-plugin-offtopic/client/containers/OffTopicTag.js b/plugins/talk-plugin-offtopic/client/containers/OffTopicTag.js new file mode 100644 index 000000000..bf57dc5cb --- /dev/null +++ b/plugins/talk-plugin-offtopic/client/containers/OffTopicTag.js @@ -0,0 +1,15 @@ +import {gql} from 'react-apollo'; +import OffTopicTag from '../components/OffTopicTag'; +import {withFragments} from 'plugin-api/beta/client/hocs'; + +export default withFragments({ + comment: gql` + fragment TalkOfftopic_OffTopicTag_comment on Comment { + tags { + tag { + name + } + } + } + ` +})(OffTopicTag); diff --git a/plugins/talk-plugin-offtopic/client/index.js b/plugins/talk-plugin-offtopic/client/index.js index b284f7280..24bfc095f 100644 --- a/plugins/talk-plugin-offtopic/client/index.js +++ b/plugins/talk-plugin-offtopic/client/index.js @@ -1,5 +1,5 @@ import translations from './translations.json'; -import OffTopicTag from './components/OffTopicTag'; +import OffTopicTag from './containers/OffTopicTag'; import OffTopicFilter from './containers/OffTopicFilter'; import OffTopicCheckbox from './containers/OffTopicCheckbox'; import reducer from './reducer'; From de90bdbd19c4bc4840d79f29946b59c7ef8ba25e Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 21:19:13 +0700 Subject: [PATCH 23/40] Gracefully handle when data is not available --- client/coral-framework/hocs/withFragments.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/coral-framework/hocs/withFragments.js b/client/coral-framework/hocs/withFragments.js index 33c39abd0..e6aca318a 100644 --- a/client/coral-framework/hocs/withFragments.js +++ b/client/coral-framework/hocs/withFragments.js @@ -30,7 +30,9 @@ function filterProps(props, fragments) { if (!(key in props)) { return; } - filtered[key] = filter(fragments[key], props[key], props.data.variables); + filtered[key] = props.data + ? filter(fragments[key], props[key], props.data.variables) + : props[key]; }); return filtered; } From e5156f7fc1895ef826ae58d7651a218bb58d254a Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 21:36:14 +0700 Subject: [PATCH 24/40] Integrate embed slot in to graphql framework --- client/coral-embed-stream/src/components/Embed.js | 10 +++++++--- client/coral-embed-stream/src/containers/Embed.js | 7 ++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/client/coral-embed-stream/src/components/Embed.js b/client/coral-embed-stream/src/components/Embed.js index f918b7a16..a74028627 100644 --- a/client/coral-embed-stream/src/components/Embed.js +++ b/client/coral-embed-stream/src/components/Embed.js @@ -28,7 +28,7 @@ export default class Embed extends React.Component { }; render() { - const {activeTab, commentId, auth: {showSignInDialog, signInDialogFocus}, blurSignInDialog, focusSignInDialog, hideSignInDialog} = this.props; + const {activeTab, commentId, root, data, auth: {showSignInDialog, signInDialogFocus}, blurSignInDialog, focusSignInDialog, hideSignInDialog} = this.props; const {user} = this.props.auth; const hasHighlightedComment = !!commentId; @@ -64,14 +64,18 @@ export default class Embed extends React.Component { } - + - + diff --git a/client/coral-embed-stream/src/containers/Embed.js b/client/coral-embed-stream/src/containers/Embed.js index e21a683d1..731cab264 100644 --- a/client/coral-embed-stream/src/containers/Embed.js +++ b/client/coral-embed-stream/src/containers/Embed.js @@ -11,7 +11,7 @@ import {Spinner} from 'coral-ui'; import * as authActions from 'coral-framework/actions/auth'; import * as assetActions from 'coral-framework/actions/asset'; import pym from 'coral-framework/services/pym'; -import {getDefinitionName} from 'coral-framework/utils'; +import {getDefinitionName, getSlotFragmentSpreads} from 'coral-framework/utils'; import {withQuery} from 'coral-framework/hocs'; import Embed from '../components/Embed'; import Stream from './Stream'; @@ -146,12 +146,17 @@ const USERNAME_REJECTED_SUBSCRIPTION = gql` } `; +const slots = [ + 'embed', +]; + const EMBED_QUERY = gql` query CoralEmbedStream_Embed($assetId: ID, $assetUrl: String, $commentId: ID!, $hasComment: Boolean!, $excludeIgnored: Boolean) { me { id status } + ${getSlotFragmentSpreads(slots, 'root')} ...${getDefinitionName(Stream.fragments.root)} } ${Stream.fragments.root} From 64580696fb8d620e7c084cf6b9604296ce68bfd9 Mon Sep 17 00:00:00 2001 From: Belen Curcio Date: Fri, 18 Aug 2017 11:42:23 -0300 Subject: [PATCH 25/40] Reliability Added --- client/coral-admin/src/components/UserDetail.js | 6 +++++- client/coral-admin/src/containers/UserDetail.js | 3 +++ client/coral-framework/utils/user.js | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 client/coral-framework/utils/user.js diff --git a/client/coral-admin/src/components/UserDetail.js b/client/coral-admin/src/components/UserDetail.js index 64018cde8..6af1b6678 100644 --- a/client/coral-admin/src/components/UserDetail.js +++ b/client/coral-admin/src/components/UserDetail.js @@ -9,6 +9,8 @@ import {actionsMap} from '../utils/moderationQueueActionsMap'; import ClickOutside from 'coral-framework/components/ClickOutside'; import LoadMore from '../components/LoadMore'; import cn from 'classnames'; +import capitalize from 'lodash/capitalize'; +import {getReliability} from 'coral-framework/utils/user'; export default class UserDetail extends React.Component { @@ -116,7 +118,9 @@ export default class UserDetail extends React.Component {
  • Reports - Reliable + + {capitalize(getReliability(user.reliable.flagger))} +
    • diff --git a/client/coral-admin/src/containers/UserDetail.js b/client/coral-admin/src/containers/UserDetail.js index 2ae6e4c9c..41fccb7c5 100644 --- a/client/coral-admin/src/containers/UserDetail.js +++ b/client/coral-admin/src/containers/UserDetail.js @@ -142,6 +142,9 @@ export const withUserDetailQuery = withQuery(gql` id provider } + reliable { + flagger + } ${getSlotFragmentSpreads(slots, 'user')} } totalComments: commentCount(query: {author_id: $author_id}) diff --git a/client/coral-framework/utils/user.js b/client/coral-framework/utils/user.js new file mode 100644 index 000000000..044583026 --- /dev/null +++ b/client/coral-framework/utils/user.js @@ -0,0 +1,14 @@ + /** + * getReliability + * retrieves reliability value as string + */ + +export const getReliability = (reliabilityValue) => { + if (reliabilityValue === null) { + return 'neutral'; + } else if (reliabilityValue) { + return 'reliable'; + } else { + return 'unreliable'; + } +}; From 833e78f99a6a40cef12183c3a823071532a5cc83 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 21:41:46 +0700 Subject: [PATCH 26/40] Integrate ProfileContainer slot into graphql framework --- client/coral-settings/containers/ProfileContainer.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/client/coral-settings/containers/ProfileContainer.js b/client/coral-settings/containers/ProfileContainer.js index 6ffad3c87..f1b3787dd 100644 --- a/client/coral-settings/containers/ProfileContainer.js +++ b/client/coral-settings/containers/ProfileContainer.js @@ -14,6 +14,7 @@ import CommentHistory from 'talk-plugin-history/CommentHistory'; import {showSignInDialog, checkLogin} from 'coral-framework/actions/auth'; import {insertCommentsSorted} from 'plugin-api/beta/client/utils'; import update from 'immutability-helper'; +import {getSlotFragmentSpreads} from 'coral-framework/utils'; import t from 'coral-framework/services/i18n'; @@ -94,6 +95,11 @@ class ProfileContainer extends Component { } } +// TODO: This Slot should be included in `talk-plugin-history` instead. +const slots = [ + 'commentContent', +]; + const CommentFragment = gql` fragment TalkSettings_CommentConnectionFragment on CommentConnection { nodes { @@ -103,8 +109,10 @@ const CommentFragment = gql` id title url + ${getSlotFragmentSpreads(slots, 'asset')} } created_at + ${getSlotFragmentSpreads(slots, 'comment')} } endCursor hasNextPage @@ -133,6 +141,7 @@ const withProfileQuery = withQuery( ...TalkSettings_CommentConnectionFragment } } + ${getSlotFragmentSpreads(slots, 'root')} } ${CommentFragment} `); From 0cbf6e24f77c64b56208f2313cabe7c93445a6b3 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 21:59:43 +0700 Subject: [PATCH 27/40] Handle null values --- client/coral-framework/helpers/plugins.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/coral-framework/helpers/plugins.js b/client/coral-framework/helpers/plugins.js index 8fc3310e6..57caf2c46 100644 --- a/client/coral-framework/helpers/plugins.js +++ b/client/coral-framework/helpers/plugins.js @@ -50,6 +50,11 @@ function withWarnings(component, queryData) { // Show warnings when accessing queryData only when not in production. return mapValues(queryData, (value, key) => { + + // Keep null values.. + if (!queryData[key]) { + return queryData[key]; + } return new Proxy(queryData[key], { get(target, name) { From b8452b535bfdc3fb486d759d3d273abd1e2ac8e6 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 22:19:03 +0700 Subject: [PATCH 28/40] Allow passing custom fragments to withReaction and withTag --- plugin-api/beta/client/hocs/withReaction.js | 20 +++++++++++++++---- plugin-api/beta/client/hocs/withTags.js | 20 +++++++++++++++---- .../client/containers/Comment.js | 1 - .../client/containers/ModTag.js | 13 +++++++++++- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/plugin-api/beta/client/hocs/withReaction.js b/plugin-api/beta/client/hocs/withReaction.js index 0d118a12f..158431898 100644 --- a/plugin-api/beta/client/hocs/withReaction.js +++ b/plugin-api/beta/client/hocs/withReaction.js @@ -13,13 +13,17 @@ import {capitalize} from 'coral-framework/helpers/strings'; import {getMyActionSummary, getTotalActionCount} from 'coral-framework/utils'; import hoistStatics from 'recompose/hoistStatics'; import * as PropTypes from 'prop-types'; +import {getDefinitionName} from '../utils'; -export default (reaction) => hoistStatics((WrappedComponent) => { +export default (reaction, options = {}) => hoistStatics((WrappedComponent) => { if (typeof reaction !== 'string') { console.error('Reaction must be a valid string'); return null; } + // fragments allow the extension of the fragments defined in this HOC. + const {fragments = {}} = options; + // Global instance counter for each `reaction` type. let instances = 0; @@ -248,7 +252,7 @@ export default (reaction) => hoistStatics((WrappedComponent) => { } render() { - const {comment} = this.props; + const {root, asset, comment} = this.props; const reactionSummary = getMyActionSummary( `${Reaction}ActionSummary`, @@ -263,10 +267,12 @@ export default (reaction) => hoistStatics((WrappedComponent) => { const alreadyReacted = !!reactionSummary; return hoistStatics((WrappedComponent) => { const enhance = compose( withFragments({ + ...fragments, asset: gql` fragment ${Reaction}Button_asset on Asset { id + ${fragments.asset ? `...${getDefinitionName(fragments.asset)}` : ''} } + ${fragments.asset ? fragments.asset : ''} `, comment: gql` fragment ${Reaction}Button_comment on Comment { @@ -389,7 +398,10 @@ export default (reaction) => hoistStatics((WrappedComponent) => { } } } - }` + ${fragments.comment ? `...${getDefinitionName(fragments.comment)}` : ''} + } + ${fragments.comment ? fragments.comment : ''} + ` }), connect(mapStateToProps, mapDispatchToProps), withDeleteReaction, diff --git a/plugin-api/beta/client/hocs/withTags.js b/plugin-api/beta/client/hocs/withTags.js index b1d565557..d13fbc8c2 100644 --- a/plugin-api/beta/client/hocs/withTags.js +++ b/plugin-api/beta/client/hocs/withTags.js @@ -9,13 +9,17 @@ import withFragments from 'coral-framework/hocs/withFragments'; import {addNotification} from 'coral-framework/actions/notification'; import {forEachError, isTagged} from 'coral-framework/utils'; import hoistStatics from 'recompose/hoistStatics'; +import {getDefinitionName} from '../utils'; -export default (tag) => hoistStatics((WrappedComponent) => { +export default (tag, options = {}) => hoistStatics((WrappedComponent) => { if (typeof tag !== 'string') { console.error('Tag must be a valid string'); return null; } + // fragments allow the extension of the fragments defined in this HOC. + const {fragments = {}} = options; + const Tag = capitalize(tag); const TAG = tag.toUpperCase(); @@ -69,13 +73,15 @@ export default (tag) => hoistStatics((WrappedComponent) => { } render() { - const {comment, user, config} = this.props; + const {root, asset, comment, user, config} = this.props; const alreadyTagged = isTagged(comment.tags, TAG); return hoistStatics((WrappedComponent) => { const enhance = compose( withFragments({ + ...fragments, asset: gql` fragment ${Tag}Button_asset on Asset { id + ${fragments.asset ? `...${getDefinitionName(fragments.asset)}` : ''} } + ${fragments.asset ? fragments.asset : ''} `, comment: gql` fragment ${Tag}Button_comment on Comment { @@ -106,7 +115,10 @@ export default (tag) => hoistStatics((WrappedComponent) => { name } } - }` + ${fragments.comment ? `...${getDefinitionName(fragments.comment)}` : ''} + } + ${fragments.comment ? fragments.comment : ''} + ` }), connect(mapStateToProps, mapDispatchToProps), withAddTag, diff --git a/plugins/talk-plugin-featured-comments/client/containers/Comment.js b/plugins/talk-plugin-featured-comments/client/containers/Comment.js index 7080627e6..bba764a1d 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/Comment.js +++ b/plugins/talk-plugin-featured-comments/client/containers/Comment.js @@ -31,7 +31,6 @@ export default withFragments({ name } } - user { id username diff --git a/plugins/talk-plugin-featured-comments/client/containers/ModTag.js b/plugins/talk-plugin-featured-comments/client/containers/ModTag.js index 3b564d127..7081c7ec6 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/ModTag.js +++ b/plugins/talk-plugin-featured-comments/client/containers/ModTag.js @@ -1,5 +1,16 @@ import ModTag from '../components/ModTag'; import {withTags} from 'plugin-api/beta/client/hocs'; +import {gql} from 'react-apollo'; -export default withTags('featured')(ModTag); +const fragments = { + comment: gql` + fragment TalkFeaturedComments_ModTag_comment on Comment { + user { + username + } + } + ` +}; + +export default withTags('featured', {fragments})(ModTag); From 0a602b79c3a1efeeecc0f15a76b04d03e2806082 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 22:21:51 +0700 Subject: [PATCH 29/40] Enable optimization for more slots --- .../coral-admin/src/components/UserDetail.js | 4 +- .../routes/Moderation/components/Comment.js | 42 ++++++++++--------- .../Moderation/components/Moderation.js | 3 +- client/talk-plugin-history/Comment.js | 4 +- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/client/coral-admin/src/components/UserDetail.js b/client/coral-admin/src/components/UserDetail.js index 34773ddef..afdccb634 100644 --- a/client/coral-admin/src/components/UserDetail.js +++ b/client/coral-admin/src/components/UserDetail.js @@ -56,6 +56,7 @@ export default class UserDetail extends React.Component { renderLoaded() { const { + root, root: { user, totalComments, @@ -101,8 +102,7 @@ export default class UserDetail extends React.Component {

    Member since {new Date(user.created_at).toLocaleString()}

    diff --git a/client/coral-admin/src/routes/Moderation/components/Comment.js b/client/coral-admin/src/routes/Moderation/components/Comment.js index c6ec0fd4c..60077e80a 100644 --- a/client/coral-admin/src/routes/Moderation/components/Comment.js +++ b/client/coral-admin/src/routes/Moderation/components/Comment.js @@ -29,7 +29,12 @@ class Comment extends React.Component { bannedWords, selected, className, - ...props + data, + root, + currentUserId, + currentAsset, + acceptComment, + rejectComment, } = this.props; const flagActionSummaries = getActionSummary('FlagActionSummary', comment); @@ -38,20 +43,22 @@ class Comment extends React.Component { let selectionStateCSS = selected ? 'mdl-shadow--16dp' : 'mdl-shadow--2dp'; - const showSuspenUserDialog = () => props.showSuspendUserDialog({ + const showSuspenUserDialog = () => this.props.showSuspendUserDialog({ userId: comment.user.id, username: comment.user.username, commentId: comment.id, commentStatus: comment.status, }); - const showBanUserDialog = () => props.showBanUserDialog({ + const showBanUserDialog = () => this.props.showBanUserDialog({ userId: comment.user.id, username: comment.user.username, commentId: comment.id, commentStatus: comment.status, }); + const queryData = {root, comment, asset: comment.asset}; + return (
  •  ({t('comment.edited')}) : null } - {props.currentUserId !== comment.user.id && + {currentUserId !== comment.user.id &&
    • @@ -103,7 +108,7 @@ class Comment extends React.Component {
    Story: {comment.asset.title} - {!props.currentAsset && + {!currentAsset && {t('modqueue.moderate')}}
    @@ -124,10 +129,9 @@ class Comment extends React.Component {

    @@ -150,30 +154,28 @@ class Comment extends React.Component { acceptComment={() => (comment.status === 'ACCEPTED' ? null - : props.acceptComment({commentId: comment.id}))} + : acceptComment({commentId: comment.id}))} rejectComment={() => (comment.status === 'REJECTED' ? null - : props.rejectComment({commentId: comment.id}))} + : rejectComment({commentId: comment.id}))} /> ); })}
    {flagActions && flagActions.length ?

    Date: Fri, 18 Aug 2017 22:35:13 +0700 Subject: [PATCH 30/40] Only use Proxy when available --- client/coral-framework/helpers/plugins.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/coral-framework/helpers/plugins.js b/client/coral-framework/helpers/plugins.js index 57caf2c46..3173115c2 100644 --- a/client/coral-framework/helpers/plugins.js +++ b/client/coral-framework/helpers/plugins.js @@ -45,8 +45,10 @@ export function isSlotEmpty(slot, reduxState, props = {}, queryData = {}) { // Memoize the warnings so we only show them once. const memoizedWarnings = []; +// withWarnings decorates the props of queryData with a proxy that +// prints a warning when accessing deeper props. function withWarnings(component, queryData) { - if (process.env.NODE_ENV !== 'production') { + if (process.env.NODE_ENV !== 'production' && window.Proxy) { // Show warnings when accessing queryData only when not in production. return mapValues(queryData, (value, key) => { From e77a8ce3c7bdda5ac2bdadf33ef345acf4ac6fae Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 22:36:13 +0700 Subject: [PATCH 31/40] More comments --- client/coral-framework/components/Slot.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/coral-framework/components/Slot.js b/client/coral-framework/components/Slot.js index 838382978..c0cca143c 100644 --- a/client/coral-framework/components/Slot.js +++ b/client/coral-framework/components/Slot.js @@ -51,6 +51,8 @@ class Slot extends React.Component { Slot.propTypes = { fill: React.PropTypes.string.isRequired, + + // props coming from graphql must be passed through this property. queryData: React.PropTypes.object, }; From 44ec469fa5c146fed1e819627ce23ef06f0c06ff Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 22:47:30 +0700 Subject: [PATCH 32/40] Disable fragment warnings --- plugin-api/beta/client/hocs/withReaction.js | 10 ++++++++++ plugin-api/beta/client/hocs/withTags.js | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/plugin-api/beta/client/hocs/withReaction.js b/plugin-api/beta/client/hocs/withReaction.js index 158431898..3300ac2d3 100644 --- a/plugin-api/beta/client/hocs/withReaction.js +++ b/plugin-api/beta/client/hocs/withReaction.js @@ -15,6 +15,16 @@ import hoistStatics from 'recompose/hoistStatics'; import * as PropTypes from 'prop-types'; import {getDefinitionName} from '../utils'; +/* + * Disable false-positive warning below, as it doesn't work well with how we currently + * assemble the queries. + * + * Warning: fragment with name {fragment name} already exists. + * graphql-tag enforces all fragment names across your application to be unique; read more about + * this in the docs: http://dev.apollodata.com/core/fragments.html#unique-names + */ +gql.disableFragmentWarnings(); + export default (reaction, options = {}) => hoistStatics((WrappedComponent) => { if (typeof reaction !== 'string') { console.error('Reaction must be a valid string'); diff --git a/plugin-api/beta/client/hocs/withTags.js b/plugin-api/beta/client/hocs/withTags.js index d13fbc8c2..491c35038 100644 --- a/plugin-api/beta/client/hocs/withTags.js +++ b/plugin-api/beta/client/hocs/withTags.js @@ -11,6 +11,16 @@ import {forEachError, isTagged} from 'coral-framework/utils'; import hoistStatics from 'recompose/hoistStatics'; import {getDefinitionName} from '../utils'; +/* + * Disable false-positive warning below, as it doesn't work well with how we currently + * assemble the queries. + * + * Warning: fragment with name {fragment name} already exists. + * graphql-tag enforces all fragment names across your application to be unique; read more about + * this in the docs: http://dev.apollodata.com/core/fragments.html#unique-names + */ +gql.disableFragmentWarnings(); + export default (tag, options = {}) => hoistStatics((WrappedComponent) => { if (typeof tag !== 'string') { console.error('Tag must be a valid string'); From 5028f4adf41b95efcede07a9dcc1c8939ec703c8 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 23:19:10 +0700 Subject: [PATCH 33/40] Better shouldComponentUpdate detection in withFragments --- client/coral-framework/hocs/withFragments.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/coral-framework/hocs/withFragments.js b/client/coral-framework/hocs/withFragments.js index e6aca318a..9bc06290e 100644 --- a/client/coral-framework/hocs/withFragments.js +++ b/client/coral-framework/hocs/withFragments.js @@ -86,9 +86,9 @@ export default (fragments) => hoistStatics((BaseComponent) => { } shouldComponentUpdate(next) { + const onlyQueryDataChanges = this.shallowChanges.every((key) => this.fragmentKeys.indexOf(key) >= 0); - // If only query data was changed. - if (this.queryDataHasChanged && this.shallowChanges.every((key) => this.fragmentKeys.indexOf(key) >= 0)) { + if (onlyQueryDataChanges) { return this.queryDataHasChanged; } From 58a5d5eea835297b2cece69a9e82973e0e854521 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 23:20:05 +0700 Subject: [PATCH 34/40] Refactor Moderation Comment a little bit --- .../routes/Moderation/components/Comment.js | 45 ++++++++++++------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/client/coral-admin/src/routes/Moderation/components/Comment.js b/client/coral-admin/src/routes/Moderation/components/Comment.js index 60077e80a..8d543e97b 100644 --- a/client/coral-admin/src/routes/Moderation/components/Comment.js +++ b/client/coral-admin/src/routes/Moderation/components/Comment.js @@ -20,6 +20,31 @@ import t, {timeago} from 'coral-framework/services/i18n'; class Comment extends React.Component { + showSuspenUserDialog = () => { + const {comment, showSuspendUserDialog} = this.props; + return showSuspendUserDialog({ + userId: comment.user.id, + username: comment.user.username, + commentId: comment.id, + commentStatus: comment.status, + }); + }; + + showBanUserDialog = () => { + const {comment, showBanUserDialog} = this.props; + return showBanUserDialog({ + userId: comment.user.id, + username: comment.user.username, + commentId: comment.id, + commentStatus: comment.status, + }); + }; + + viewUserDetail = () => { + const {viewUserDetail, comment} = this.props; + return viewUserDetail(comment.user.id); + }; + render() { const { actions = [], @@ -43,20 +68,6 @@ class Comment extends React.Component { let selectionStateCSS = selected ? 'mdl-shadow--16dp' : 'mdl-shadow--2dp'; - const showSuspenUserDialog = () => this.props.showSuspendUserDialog({ - userId: comment.user.id, - username: comment.user.username, - commentId: comment.id, - commentStatus: comment.status, - }); - - const showBanUserDialog = () => this.props.showBanUserDialog({ - userId: comment.user.id, - username: comment.user.username, - commentId: comment.id, - commentStatus: comment.status, - }); - const queryData = {root, comment, asset: comment.asset}; return ( @@ -69,7 +80,7 @@ class Comment extends React.Component {

    { ( - viewUserDetail(comment.user.id)}> + {comment.user.username} ) @@ -86,11 +97,11 @@ class Comment extends React.Component { + onClick={this.showSuspenUserDialog}> Suspend User + onClick={this.showBanUserDialog}> Ban User From cf1c93f395411d71eb56c9d1ecb1e3e031a81864 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 18 Aug 2017 23:37:53 +0700 Subject: [PATCH 35/40] Add comments --- client/coral-embed-stream/src/containers/Comment.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/client/coral-embed-stream/src/containers/Comment.js b/client/coral-embed-stream/src/containers/Comment.js index 5f3ee0c9d..005a24ae1 100644 --- a/client/coral-embed-stream/src/containers/Comment.js +++ b/client/coral-embed-stream/src/containers/Comment.js @@ -18,6 +18,12 @@ const slots = [ 'commentAvatar' ]; +/** + * withAnimateEnter is a HOC that passes a property `animateEnter` to the + * underlying BaseComponent. It must be a direct child of a `TransitionGroup` + * from https://github.com/reactjs/react-transition-group and as such must + * be the uppermost HOC applied to the BaseComponent. + */ const withAnimateEnter = hoistStatics((BaseComponent) => { class WithAnimateEnter extends React.Component { state = { From f49591225f314b3e0b003b6e7e6680a5b624362e Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Sat, 19 Aug 2017 00:10:31 +0700 Subject: [PATCH 36/40] Use UserDetailComment container --- client/coral-admin/src/components/UserDetail.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/coral-admin/src/components/UserDetail.js b/client/coral-admin/src/components/UserDetail.js index 9ee16935f..11cd444e1 100644 --- a/client/coral-admin/src/components/UserDetail.js +++ b/client/coral-admin/src/components/UserDetail.js @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import Comment from './UserDetailComment'; +import Comment from '../containers/UserDetailComment'; import styles from './UserDetail.css'; import {Icon, Button, Drawer, Spinner} from 'coral-ui'; import {Slot} from 'coral-framework/components'; @@ -99,7 +99,7 @@ export default class UserDetail extends React.Component { {new Date(user.created_at).toLocaleString()} - {user.profiles.map(({id}) => + {user.profiles.map(({id}) =>
  • Email: From 1b09825602594913e88c7012a34b1a0535297f36 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 18 Aug 2017 11:50:51 -0600 Subject: [PATCH 37/40] added more debugging to redis, improved retry --- config.js | 18 +++++++++++ docs/_docs/02-01-configuration.md | 15 +++++++++ graph/context.js | 2 +- graph/subscriptions.js | 4 +-- middleware/pubsub.js | 3 +- services/pubsub.js | 29 +++++++++-------- services/redis.js | 54 +++++++++++++++++++++++++------ 7 files changed, 96 insertions(+), 29 deletions(-) diff --git a/config.js b/config.js index a92802ee7..22720a36a 100644 --- a/config.js +++ b/config.js @@ -8,6 +8,7 @@ require('env-rewrite').rewrite(); const uniq = require('lodash/uniq'); +const ms = require('ms'); //============================================================================== // CONFIG INITIALIZATION @@ -84,6 +85,23 @@ const CONFIG = { MONGO_URL: process.env.TALK_MONGO_URL, REDIS_URL: process.env.TALK_REDIS_URL, + // REDIS_RECONNECTION_MAX_ATTEMPTS is the amount of attempts that a redis + // connection will attempt to reconnect before aborting with an error. + REDIS_RECONNECTION_MAX_ATTEMPTS: parseInt(process.env.TALK_REDIS_RECONNECTION_MAX_ATTEMPTS || '100'), + + // REDIS_RECONNECTION_MAX_RETRY_TIME is the time in string format for the + // maximum amount of time that a client can be considered "connecting" before + // attempts at reconnection are aborted with an error. + REDIS_RECONNECTION_MAX_RETRY_TIME: ms(process.env.TALK_REDIS_RECONNECTION_MAX_RETRY_TIME || '1 min'), + + // REDIS_RECONNECTION_BACKOFF_FACTOR is the factor that will be multiplied + // against the current attempt count inbetween attempts to connect to redis. + REDIS_RECONNECTION_BACKOFF_FACTOR: ms(process.env.TALK_REDIS_RECONNECTION_BACKOFF_FACTOR || '500 ms'), + + // REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME is the minimum time used to delay + // before attempting to reconnect to redis. + REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME: ms(process.env.TALK_REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME || '1 sec'), + //------------------------------------------------------------------------------ // Server Config //------------------------------------------------------------------------------ diff --git a/docs/_docs/02-01-configuration.md b/docs/_docs/02-01-configuration.md index 8f76ad999..58c828f9a 100644 --- a/docs/_docs/02-01-configuration.md +++ b/docs/_docs/02-01-configuration.md @@ -52,6 +52,21 @@ These are only used during the webpack build. - `TALK_MONGO_URL` (*required*) - the database connection string for the MongoDB database. - `TALK_REDIS_URL` (*required*) - the database connection string for the Redis database. +#### Advanced + +- `TALK_REDIS_RECONNECTION_MAX_ATTEMPTS` (_optional_) - the amount of attempts + that a redis connection will attempt to reconnect before aborting with an + error. (Default `100`) +- `TALK_REDIS_RECONNECTION_MAX_RETRY_TIME` (_optional_) - the time in string + format for the maximum amount of time that a client can be considered + "connecting" before attempts at reconnection are aborted with an error. + (Default `1 min`) +- `TALK_REDIS_RECONNECTION_BACKOFF_FACTOR` (_optional_) - the time factor that + will be multiplied against the current attempt count inbetween attempts to + connect to redis. (Default `500 ms`) +- `TALK_REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME` (_optional_) - the minimum time + used to delay before attempting to reconnect to redis. (Default `1 sec`) + ### Server - `TALK_ROOT_URL` (*required*) - root url of the installed application externally diff --git a/graph/context.js b/graph/context.js index cffec0896..8d7c740b6 100644 --- a/graph/context.js +++ b/graph/context.js @@ -53,7 +53,7 @@ class Context { this.plugins = decorateContextPlugins(this, contextPlugins); // Bind the publish/subscribe to the context. - this.pubsub = pubsub.createClient(); + this.pubsub = pubsub.getClient(); } } diff --git a/graph/subscriptions.js b/graph/subscriptions.js index adac849e0..f89b41d3a 100644 --- a/graph/subscriptions.js +++ b/graph/subscriptions.js @@ -127,15 +127,13 @@ const setupFunctions = plugins.get('server', 'setupFunctions').reduce((acc, {plu }), }); -const pubsubClient = pubsub.createClientFactory(); - /** * This creates a new subscription manager. */ const createSubscriptionManager = (server) => new SubscriptionServer({ subscriptionManager: new SubscriptionManager({ schema, - pubsub: pubsubClient(), + pubsub: pubsub.getClient(), setupFunctions, }), onConnect: ({token}, connection) => { diff --git a/middleware/pubsub.js b/middleware/pubsub.js index 15f7c51a2..957a523a7 100644 --- a/middleware/pubsub.js +++ b/middleware/pubsub.js @@ -1,12 +1,11 @@ const pubsub = require('../services/pubsub'); -const pubsubClient = pubsub.createClientFactory(); // To handle dependancy injection safer, we inject the pubsub handle onto the // request object. module.exports = (req, res, next) => { // Attach the pubsub handle to the requests. - req.pubsub = pubsubClient(); + req.pubsub = pubsub.getClient(); // Forward on the request. next(); diff --git a/services/pubsub.js b/services/pubsub.js index 780aeba16..9fc2ae902 100644 --- a/services/pubsub.js +++ b/services/pubsub.js @@ -1,23 +1,24 @@ const {RedisPubSub} = require('graphql-redis-subscriptions'); +const {connectionOptions, attachMonitors} = require('./redis'); -const {connectionOptions} = require('./redis'); +/** + * getClient returns the pubsub singleton for this instance. + */ +let pubsub = null; +const getClient = () => { + if (pubsub !== null) { + return pubsub; + } -const createClient = () => new RedisPubSub({connection: connectionOptions}); + pubsub = new RedisPubSub({connection: connectionOptions}); -const createClientFactory = () => { - let ins = null; - return () => { - if (ins) { - return ins; - } + // Attach the node monitors to the subscriber + publishers. + attachMonitors(pubsub.redisPublisher); + attachMonitors(pubsub.redisSubscriber); - ins = createClient(); - - return ins; - }; + return pubsub; }; module.exports = { - createClient, - createClientFactory + getClient, }; diff --git a/services/redis.js b/services/redis.js index 1983249f1..b87e5e9fd 100644 --- a/services/redis.js +++ b/services/redis.js @@ -1,45 +1,80 @@ const redis = require('redis'); const debug = require('debug')('talk:services:redis'); +const enabled = require('debug').enabled('talk:services:redis'); const { - REDIS_URL + REDIS_URL, + REDIS_RECONNECTION_MAX_ATTEMPTS, + REDIS_RECONNECTION_MAX_RETRY_TIME, + REDIS_RECONNECTION_BACKOFF_FACTOR, + REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME, } = require('../config'); +const attachMonitors = (client) => { + debug('client created'); + + // Debug events. + if (enabled) { + client.on('ready', () => debug('client ready')); + client.on('connect', () => debug('client connected')); + client.on('reconnecting', () => debug('client connection lost, attempting to reconnect')); + client.on('end', () => debug('client ended')); + } + + // Error events. + client.on('error', (err) => { + if (err) { + console.error('Error connecting to redis:', err); + } + }); +}; + const connectionOptions = { url: REDIS_URL, retry_strategy: function(options) { - if (options.error && options.error.code === 'ECONNREFUSED') { + if (options.error && options.error.code !== 'ECONNREFUSED') { + + debug('retry strategy: none, an error occured'); // End reconnecting on a specific error and flush all commands with a individual error - return new Error('The server refused the connection'); + return options.error; } - if (options.total_retry_time > 1000 * 60 * 60) { + if (options.total_retry_time > REDIS_RECONNECTION_MAX_RETRY_TIME) { + + debug('retry strategy: none, exhausted retry time'); // End reconnecting after a specific timeout and flush all commands with a individual error return new Error('Retry time exhausted'); } - if (options.times_connected > 10) { + if (options.attempt > REDIS_RECONNECTION_MAX_ATTEMPTS) { + + debug('retry strategy: none, exhausted retry attempts'); // End reconnecting with built in error return undefined; } // reconnect after - return Math.max(options.attempt * 100, 3000); + const delay = Math.max(options.attempt * REDIS_RECONNECTION_BACKOFF_FACTOR, REDIS_RECONNECTION_BACKOFF_MINIMUM_TIME); + + debug(`retry strategy: try to reconnect ${delay} ms from now`); + + return delay; } }; const createClient = () => { let client = redis.createClient(connectionOptions); + // Attach the monitors that will print debug messages to the console. + attachMonitors(client); + client.ping((err) => { if (err) { console.error('Can\'t ping the redis server!'); throw err; } - - debug('connection established'); }); return client; @@ -47,12 +82,13 @@ const createClient = () => { module.exports = { connectionOptions, + attachMonitors, createClient, createClientFactory: () => { let client = null; return () => { - if (client) { + if (client !== null) { return client; } From 3b26e855bc891fe7f4412e4eaa9edfefc1b70ce9 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Mon, 21 Aug 2017 17:02:54 +0700 Subject: [PATCH 38/40] Don't manipulate state directly... --- .../containers/ConfigureStreamContainer.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/client/coral-configure/containers/ConfigureStreamContainer.js b/client/coral-configure/containers/ConfigureStreamContainer.js index 8489c809f..1db8f497a 100644 --- a/client/coral-configure/containers/ConfigureStreamContainer.js +++ b/client/coral-configure/containers/ConfigureStreamContainer.js @@ -15,7 +15,7 @@ class ConfigureStreamContainer extends Component { this.state = { changed: false, - dirtySettings: props.asset.settings, + dirtySettings: {...props.asset.settings}, closedAt: !props.asset.isClosed ? 'open' : 'closed' }; @@ -48,26 +48,28 @@ class ConfigureStreamContainer extends Component { changed: false }); }, 300); - - // this.props.loadAsset(this.props.data.asset); } } handleChange (e) { + const changes = {}; - // TODO: Don’t directly manipulate state and make state change immutable. if (e.target && e.target.id === 'qboxenable') { - this.state.dirtySettings.questionBoxEnable = e.target.checked; + changes.questionBoxEnable = e.target.checked; } if (e.target && e.target.id === 'qboxcontent') { - this.state.dirtySettings.questionBoxContent = e.target.value; + changes.questionBoxContent = e.target.value; } if (e.target && e.target.id === 'plinksenable') { - this.state.dirtySettings.premodLinksEnable = e.target.value; + changes.premodLinksEnable = e.target.value; } this.setState({ - changed: true + changed: true, + dirtySettings: { + ...this.state.dirtySettings, + ...changes, + }, }); } From 077bc5ad3352665843eb97af8501c912ec9ec2ac Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Mon, 21 Aug 2017 17:09:15 +0700 Subject: [PATCH 39/40] Fix typo --- .../coral-admin/src/routes/Moderation/components/Comment.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/coral-admin/src/routes/Moderation/components/Comment.js b/client/coral-admin/src/routes/Moderation/components/Comment.js index 8d543e97b..1335a8c52 100644 --- a/client/coral-admin/src/routes/Moderation/components/Comment.js +++ b/client/coral-admin/src/routes/Moderation/components/Comment.js @@ -20,7 +20,7 @@ import t, {timeago} from 'coral-framework/services/i18n'; class Comment extends React.Component { - showSuspenUserDialog = () => { + showSuspendUserDialog = () => { const {comment, showSuspendUserDialog} = this.props; return showSuspendUserDialog({ userId: comment.user.id, @@ -97,7 +97,7 @@ class Comment extends React.Component { + onClick={this.showSuspendUserDialog}> Suspend User Date: Mon, 21 Aug 2017 08:12:36 -0600 Subject: [PATCH 40/40] changed trust defaults --- config.js | 2 +- docs/_docs/02-01-configuration.md | 2 +- services/karma.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config.js b/config.js index 22720a36a..11c8b5077 100644 --- a/config.js +++ b/config.js @@ -153,7 +153,7 @@ const CONFIG = { DISABLE_AUTOFLAG_SUSPECT_WORDS: process.env.TALK_DISABLE_AUTOFLAG_SUSPECT_WORDS === 'TRUE', // TRUST_THRESHOLDS defines the thresholds used for automoderation. - TRUST_THRESHOLDS: process.env.TRUST_THRESHOLDS || 'comment:-1,-1;flag:-1,-1' + TRUST_THRESHOLDS: process.env.TRUST_THRESHOLDS || 'comment:2,-1;flag:2,-1' }; //============================================================================== diff --git a/docs/_docs/02-01-configuration.md b/docs/_docs/02-01-configuration.md index 58c828f9a..df790b24b 100644 --- a/docs/_docs/02-01-configuration.md +++ b/docs/_docs/02-01-configuration.md @@ -177,7 +177,7 @@ Trust can auto-moderate comments based on user history. By specifying this option, the behavior can be changed to offer different results. - `TRUST_THRESHOLDS` (_optional_) - configure the reliability thresholds for - flagging and commenting. (Default `comment:-1,-1;flag:-1,-1`) + flagging and commenting. (Default `comment:2,-1;flag:2,-1`) The form of the environment variable: diff --git a/services/karma.js b/services/karma.js index a2c087eaa..687b67af0 100644 --- a/services/karma.js +++ b/services/karma.js @@ -19,7 +19,7 @@ const { * * The default used is: * - * comment:-1,-1;flag:-1,-1 + * comment:2,-2;flag:2,-2 */ const parseThresholds = (thresholds) => thresholds .split(';')