From 41e4be8ef61c6bcd77109b92ce12bbc76e7e733b Mon Sep 17 00:00:00 2001 From: Marek Hrabe Date: Fri, 17 Nov 2017 16:32:00 +0100 Subject: [PATCH] Editor: Revisions: Diff revisions only once (#19072) * Post Revisions: Don't compute or display diffs when the amount of content is too large (#19860) * Post Revisions: Don't compute or display diffs when the amount of content is too large Diff in the selector so it is memoized & can be used in both the list item & the diff view * Improve auto selecting first revision in the list * When post is too long to diff, just show the whole thing in the viewer * Post Revisions: short-circuit the selector when the flag is not enabled * remove the is-loading class when content is "too long" * Post Revisions: Update Tests for diffing change (#19944) * Update tests for the getPostRevisionChanges selector * Don't annotate the response in the receiveSuccess test. We're diffing on select now -- not receive. * Enable the feature flag in the test environment --- .../post-editor/editor-diff-viewer/index.jsx | 19 +- .../editor-revisions-list/index.jsx | 37 ++-- .../editor-revisions-list/item.jsx | 12 +- .../data-layer/wpcom/posts/revisions/index.js | 13 +- .../wpcom/posts/revisions/test/index.js | 8 +- .../selectors/get-post-revision-changes.js | 40 +++- .../test/get-post-revision-changes.js | 208 +++++++++++++----- config/test.json | 1 + 8 files changed, 228 insertions(+), 110 deletions(-) diff --git a/client/post-editor/editor-diff-viewer/index.jsx b/client/post-editor/editor-diff-viewer/index.jsx index d7e6e160825e61..1ef79c2b772e5e 100644 --- a/client/post-editor/editor-diff-viewer/index.jsx +++ b/client/post-editor/editor-diff-viewer/index.jsx @@ -14,7 +14,7 @@ import { get, has } from 'lodash'; /** * Internal dependencies */ -import { getPostRevisionChanges } from 'state/selectors'; +import { getPostRevision, getPostRevisionChanges } from 'state/selectors'; import EditorDiffChanges from './changes'; class EditorDiffViewer extends PureComponent { @@ -49,17 +49,25 @@ class EditorDiffViewer extends PureComponent { } render() { - const { revisionChanges } = this.props; + const { revision, revisionChanges } = this.props; const classes = classNames( 'editor-diff-viewer', { - 'is-loading': ! has( revisionChanges, 'title[0].value' ), + 'is-loading': ! ( revisionChanges.tooLong || has( revisionChanges, 'title[0].value' ) ), } ); return (

- + { revisionChanges.tooLong ? ( + revision.title + ) : ( + + ) } 

-					
+					{ revisionChanges.tooLong ? (
+						revision.content
+					) : (
+						
+					) }
); @@ -67,5 +75,6 @@ class EditorDiffViewer extends PureComponent { } export default connect( ( state, { siteId, postId, selectedRevisionId } ) => ( { + revision: getPostRevision( state, siteId, postId, selectedRevisionId, 'display' ), revisionChanges: getPostRevisionChanges( state, siteId, postId, selectedRevisionId ), } ) )( EditorDiffViewer ); diff --git a/client/post-editor/editor-revisions-list/index.jsx b/client/post-editor/editor-revisions-list/index.jsx index 52df01b18e84bc..be21236e7211c8 100644 --- a/client/post-editor/editor-revisions-list/index.jsx +++ b/client/post-editor/editor-revisions-list/index.jsx @@ -27,7 +27,12 @@ class EditorRevisionsList extends PureComponent { selectedRevisionId: PropTypes.number, }; - trySelectingTimeout = null; + trySelectingInterval = null; + + selectRevision = revisionId => { + clearInterval( this.trySelectingInterval ); + this.props.selectPostRevision( revisionId ); + }; trySelectingFirstRevision = () => { const { revisions } = this.props; @@ -38,29 +43,26 @@ class EditorRevisionsList extends PureComponent { if ( ! firstRevision.id ) { return; } - this.props.selectPostRevision( firstRevision.id ); + this.selectRevision( firstRevision.id ); }; - componentWillReceiveProps( { selectedRevisionId } ) { - if ( - ! this.trySelectingTimeout && - ( ! selectedRevisionId || ! this.props.selectedRevisionId ) - ) { - this.trySelectingTimeout = setTimeout( this.trySelectingFirstRevision, 300 ); - } - } - componentDidMount() { // Make sure that scroll position in the editor is not preserved. window.scrollTo( 0, 0 ); KeyboardShortcuts.on( 'move-selection-up', this.selectNextRevision ); KeyboardShortcuts.on( 'move-selection-down', this.selectPreviousRevision ); + + if ( ! this.props.selectedRevisionId ) { + this.trySelectingInterval = setInterval( this.trySelectingFirstRevision, 500 ); + } } componentWillUnmount() { KeyboardShortcuts.off( 'move-selection-up', this.selectNextRevision ); KeyboardShortcuts.off( 'move-selection-down', this.selectPreviousRevision ); + + clearInterval( this.trySelectingInterval ); } componentDidUpdate() { @@ -95,19 +97,20 @@ class EditorRevisionsList extends PureComponent { selectNextRevision = () => { const { nextRevisionId } = this.props; - nextRevisionId && this.props.selectPostRevision( nextRevisionId ); + nextRevisionId && this.selectRevision( nextRevisionId ); }; selectPreviousRevision = () => { const { prevRevisionId } = this.props; - prevRevisionId && this.props.selectPostRevision( prevRevisionId ); + prevRevisionId && this.selectRevision( prevRevisionId ); }; render() { - const { revisions, selectedRevisionId, siteId } = this.props; + const { postId, revisions, selectedRevisionId, siteId } = this.props; const classes = classNames( 'editor-revisions-list', { 'is-loading': isEmpty( revisions ), } ); + return (
@@ -119,7 +122,11 @@ class EditorRevisionsList extends PureComponent { } ); return (
  • - +
    • ); } ) } diff --git a/client/post-editor/editor-revisions-list/item.jsx b/client/post-editor/editor-revisions-list/item.jsx index c5f54b88c4dd6c..f30d99aea39cfc 100644 --- a/client/post-editor/editor-revisions-list/item.jsx +++ b/client/post-editor/editor-revisions-list/item.jsx @@ -13,6 +13,7 @@ import { flow, get } from 'lodash'; /** * Internal dependencies */ +import { getPostRevisionChanges } from 'state/selectors'; import { selectPostRevision } from 'state/posts/revisions/actions'; import { isSingleUserSite } from 'state/sites/selectors'; import TimeSince from 'components/time-since'; @@ -23,10 +24,10 @@ class EditorRevisionsListItem extends PureComponent { }; render() { - const { revision, isMultiUserSite, translate } = this.props; + const { revision, revisionChanges, isMultiUserSite, translate } = this.props; const authorName = get( revision, 'author.display_name' ); - const added = get( revision, 'changes.added' ); - const removed = get( revision, 'changes.removed' ); + const added = get( revisionChanges, 'summary.added' ); + const removed = get( revisionChanges, 'summary.removed' ); return (