diff --git a/package.json b/package.json index edd610a5b..05b9a0a86 100644 --- a/package.json +++ b/package.json @@ -78,8 +78,10 @@ "lodash.defaultsdeep": "3.10.0", "lodash.isarray": "3.0.4", "lodash.merge": "3.3.2", + "lodash.mergewith": "4.6.1", "lodash.omit": "3.1.0", "lodash.range": "3.0.1", + "lodash.uniqby": "4.7.0", "minilog": "2.0.8", "node-dir": "0.1.16", "node-sass": "4.6.1", @@ -100,7 +102,7 @@ "redux": "3.5.2", "redux-thunk": "2.0.1", "sass-loader": "6.0.6", - "scratch-gui": "0.1.0-prerelease.20190109203727", + "scratch-gui": "0.1.0-prerelease.20190111141703", "scratch-l10n": "latest", "scratchr2_translations": "git://github.com/LLK/scratchr2_translations.git#master", "slick-carousel": "1.6.0", diff --git a/src/l10n.json b/src/l10n.json index f8d8f3285..90eaeba70 100644 --- a/src/l10n.json +++ b/src/l10n.json @@ -237,7 +237,7 @@ "comments.post": "Post", "comments.cancel": "Cancel", "comments.lengthWarning": "{remainingCharacters, plural, one {1 character left} other {{remainingCharacters} characters left}}", - "comments.seeMoreReplies": "{repliesCount, plural, one {See 1 more reply} other {See all {repliesCount} replies}}", + "comments.loadMoreReplies": "See more replies", "comments.status.delbyusr": "Deleted by project owner", "comments.status.censbyfilter": "Censored by filter", "comments.status.delbyparentcomment": "Parent comment deleted", diff --git a/src/redux/preview.js b/src/redux/preview.js index 24b746ca0..12a06ba7b 100644 --- a/src/redux/preview.js +++ b/src/redux/preview.js @@ -1,11 +1,14 @@ const defaults = require('lodash.defaults'); const keyMirror = require('keymirror'); const async = require('async'); -const merge = require('lodash.merge'); +const mergeWith = require('lodash.mergewith'); +const uniqBy = require('lodash.uniqby'); const api = require('../lib/api'); const log = require('../lib/log'); +const COMMENT_LIMIT = 20; + module.exports.Status = keyMirror({ FETCHED: null, NOT_FETCHED: null, @@ -100,7 +103,7 @@ module.exports.previewReducer = (state, action) => { }); case 'SET_COMMENTS': return Object.assign({}, state, { - comments: [...state.comments, ...action.items] // TODO: consider a different way of doing this? + comments: uniqBy(state.comments.concat(action.items), 'id') }); case 'UPDATE_COMMENT': if (action.topLevelCommentId) { @@ -149,7 +152,19 @@ module.exports.previewReducer = (state, action) => { }); case 'SET_REPLIES': return Object.assign({}, state, { - replies: merge({}, state.replies, action.replies) + // Append new replies to the state.replies structure + replies: mergeWith({}, state.replies, action.replies, (replies, newReplies) => ( + uniqBy((replies || []).concat(newReplies || []), 'id') + )), + // Also set the `moreRepliesToLoad` property on the top-level comments + comments: state.comments.map(comment => { + if (action.replies[comment.id]) { + return Object.assign({}, comment, { + moreRepliesToLoad: action.replies[comment.id].length === COMMENT_LIMIT + }); + } + return comment; + }) }); case 'SET_LOVED': return Object.assign({}, state, { @@ -448,7 +463,6 @@ module.exports.getFavedStatus = (id, username, token) => (dispatch => { }); module.exports.getTopLevelComments = (id, offset, isAdmin, token) => (dispatch => { - const COMMENT_LIMIT = 20; dispatch(module.exports.setFetchStatus('comments', module.exports.Status.FETCHING)); api({ uri: `${isAdmin ? '/admin' : ''}/projects/${id}/comments`, @@ -467,7 +481,7 @@ module.exports.getTopLevelComments = (id, offset, isAdmin, token) => (dispatch = } dispatch(module.exports.setFetchStatus('comments', module.exports.Status.FETCHED)); dispatch(module.exports.setComments(body)); - dispatch(module.exports.getReplies(id, body.map(comment => comment.id), isAdmin, token)); + dispatch(module.exports.getReplies(id, body.map(comment => comment.id), 0, isAdmin, token)); // If we loaded a full page of comments, assume there are more to load. // This will be wrong (1 / COMMENT_LIMIT) of the time, but does not require @@ -503,17 +517,18 @@ module.exports.getCommentById = (projectId, commentId, isAdmin, token) => (dispa // If the comment is not a reply, show it as top level and load replies dispatch(module.exports.setFetchStatus('comments', module.exports.Status.FETCHED)); dispatch(module.exports.setComments([body])); - dispatch(module.exports.getReplies(projectId, [body.id], isAdmin, token)); + dispatch(module.exports.getReplies(projectId, [body.id], 0, isAdmin, token)); }); }); -module.exports.getReplies = (projectId, commentIds, isAdmin, token) => (dispatch => { +module.exports.getReplies = (projectId, commentIds, offset, isAdmin, token) => (dispatch => { dispatch(module.exports.setFetchStatus('replies', module.exports.Status.FETCHING)); const fetchedReplies = {}; async.eachLimit(commentIds, 10, (parentId, callback) => { api({ uri: `${isAdmin ? '/admin' : ''}/projects/${projectId}/comments/${parentId}/replies`, - authentication: isAdmin ? token : null + authentication: isAdmin ? token : null, + params: {offset: offset || 0, limit: COMMENT_LIMIT} }, (err, body) => { if (err) { return callback(`Error fetching comment replies: ${err}`); diff --git a/src/views/faq/l10n.json b/src/views/faq/l10n.json index 71617c3c0..0d9ba0167 100644 --- a/src/views/faq/l10n.json +++ b/src/views/faq/l10n.json @@ -24,7 +24,7 @@ "faq.requirementsDesktopSafari":"Safari (11+)", "faq.requirementsDesktopIE":"Internet Explorer is NOT supported.", "faq.requirementsTablet":"Tablet", - "faq.requirementsTabletChrome":"Mobile Chrome (62+)", + "faq.requirementsTabletChrome":"Mobile Chrome (63+)", "faq.requirementsTabletSafari":"Mobile Safari (11+)", "faq.requirementsNote":"Note:", "faq.requirementsNoteDesktop":"If your computer doesn’t meet these requirements, you can try the Scratch Desktop editor (see next item in FAQ). ", diff --git a/src/views/preview/comment/comment.scss b/src/views/preview/comment/comment.scss index 98e55ef6c..cff4c131b 100644 --- a/src/views/preview/comment/comment.scss +++ b/src/views/preview/comment/comment.scss @@ -235,22 +235,20 @@ .replies { width: calc(100% - 4rem); +} - &.collapsed .comment { - &:last-child { - &:after { - position: absolute; - bottom: 0; - background: linear-gradient( - $ui-light-primary-transparent, - $ui-light-primary - ); - width: 100%; - height: 100%; - content: ""; - pointer-events: none; - } - } +.replies.collapsed > .comment:last-of-type { + &:after { + position: absolute; + bottom: 0; + background: linear-gradient( + $ui-light-primary-transparent, + $ui-light-primary + ); + width: 100%; + height: 100%; + content: ""; + pointer-events: none; } } diff --git a/src/views/preview/comment/top-level-comment.jsx b/src/views/preview/comment/top-level-comment.jsx index 188150d7b..ecc15208e 100644 --- a/src/views/preview/comment/top-level-comment.jsx +++ b/src/views/preview/comment/top-level-comment.jsx @@ -28,9 +28,11 @@ class TopLevelComment extends React.Component { } handleExpandThread () { - this.setState({ - expanded: true - }); + if (this.state.expanded) { + this.props.onLoadMoreReplies(this.props.id, this.props.replies.length); + } else { + this.setState({expanded: true}); + } } handleDeleteReply (replyId) { @@ -79,6 +81,7 @@ class TopLevelComment extends React.Component { datetimeCreated, highlightedCommentId, id, + moreRepliesToLoad, onDelete, onReport, onRestore, @@ -141,21 +144,17 @@ class TopLevelComment extends React.Component { onRestore={this.handleRestoreReply} /> ))} + {((!this.state.expanded && replies.length > 3) || + (this.state.expanded && moreRepliesToLoad)) && + + + + } } - {!this.state.expanded && replies.length > 3 && - - - - } ); } @@ -178,8 +177,10 @@ TopLevelComment.propTypes = { deletable: PropTypes.bool, highlightedCommentId: PropTypes.oneOfType([PropTypes.number, PropTypes.bool]), id: PropTypes.number, + moreRepliesToLoad: PropTypes.bool, onAddComment: PropTypes.func, onDelete: PropTypes.func, + onLoadMoreReplies: PropTypes.func, onReport: PropTypes.func, onRestore: PropTypes.func, parentId: PropTypes.number, @@ -189,7 +190,8 @@ TopLevelComment.propTypes = { }; TopLevelComment.defaultProps = { - defaultExpanded: false + defaultExpanded: false, + moreRepliesToLoad: false }; module.exports = TopLevelComment; diff --git a/src/views/preview/presentation.jsx b/src/views/preview/presentation.jsx index fdccd6c16..a13ed21aa 100644 --- a/src/views/preview/presentation.jsx +++ b/src/views/preview/presentation.jsx @@ -70,6 +70,7 @@ const PreviewPresentation = ({ isFullScreen, isLoggedIn, isNewScratcher, + isProjectLoaded, isRemixing, isScratcher, isShared, @@ -88,8 +89,10 @@ const PreviewPresentation = ({ onFavoriteClicked, onGreenFlag, onLoadMore, + onLoadMoreReplies, onLoveClicked, onOpenAdminPanel, + onProjectLoaded, onRemix, onRemixing, onReportClicked, @@ -250,10 +253,11 @@ const PreviewPresentation = ({ className={classNames([ 'remix-button', { - remixing: isRemixing, - spin: isRemixing + disabled: isRemixing || !isProjectLoaded, + remixing: isRemixing } ])} + disabled={isRemixing || !isProjectLoaded} title={intl.formatMessage({id: 'project.remixButton.altText'})} onClick={onRemix} > @@ -301,6 +305,7 @@ const PreviewPresentation = ({ projectHost={projectHost} projectId={projectId} onGreenFlag={onGreenFlag} + onProjectLoaded={onProjectLoaded} onRemixing={onRemixing} onUpdateProjectId={onUpdateProjectId} onUpdateProjectThumbnail={onUpdateProjectThumbnail} @@ -550,12 +555,14 @@ const PreviewPresentation = ({ highlightedCommentId={singleCommentId} id={comment.id} key={comment.id} + moreRepliesToLoad={comment.moreRepliesToLoad} parentId={comment.parent_id} projectId={projectId} replies={replies && replies[comment.id] ? replies[comment.id] : []} visibility={comment.visibility} onAddComment={onAddComment} onDelete={onDeleteComment} + onLoadMoreReplies={onLoadMoreReplies} onReport={onReportComment} onRestore={onRestoreComment} /> @@ -623,6 +630,7 @@ PreviewPresentation.propTypes = { isFullScreen: PropTypes.bool, isLoggedIn: PropTypes.bool, isNewScratcher: PropTypes.bool, + isProjectLoaded: PropTypes.bool, isRemixing: PropTypes.bool, isScratcher: PropTypes.bool, isShared: PropTypes.bool, @@ -644,8 +652,10 @@ PreviewPresentation.propTypes = { onFavoriteClicked: PropTypes.func, onGreenFlag: PropTypes.func, onLoadMore: PropTypes.func, + onLoadMoreReplies: PropTypes.func, onLoveClicked: PropTypes.func, onOpenAdminPanel: PropTypes.func, + onProjectLoaded: PropTypes.func, onRemix: PropTypes.func, onRemixing: PropTypes.func, onReportClicked: PropTypes.func.isRequired, diff --git a/src/views/preview/preview.scss b/src/views/preview/preview.scss index 9a6fce550..7a108a33a 100644 --- a/src/views/preview/preview.scss +++ b/src/views/preview/preview.scss @@ -283,9 +283,11 @@ $stage-width: 480px; background-image: url("/svgs/project/remix-white.svg"); } } + .remix-button.disabled { + opacity: .6; + } .remix-button.remixing { - opacity: .6; &:before { animation-name: remix-intro, remix-spin; diff --git a/src/views/preview/project-view.jsx b/src/views/preview/project-view.jsx index d2512c65b..17546dc1f 100644 --- a/src/views/preview/project-view.jsx +++ b/src/views/preview/project-view.jsx @@ -60,6 +60,7 @@ class Preview extends React.Component { 'handleToggleStudio', 'handleFavoriteToggle', 'handleLoadMore', + 'handleLoadMoreReplies', 'handleLoveToggle', 'handleMessage', 'handlePopState', @@ -74,6 +75,7 @@ class Preview extends React.Component { 'handleAddToStudioClick', 'handleAddToStudioClose', 'handleGreenFlag', + 'handleProjectLoaded', 'handleRemix', 'handleSeeAllComments', 'handleSeeInside', @@ -108,6 +110,7 @@ class Preview extends React.Component { clientLoved: false, extensions: [], favoriteCount: 0, + isProjectLoaded: false, isRemixing: false, invalidProject: parts.length === 1, justRemixed: false, @@ -382,6 +385,11 @@ class Preview extends React.Component { this.props.setFullScreen(fullScreen); } } + handleProjectLoaded () { + // Currently project view only needs to know when the project becomes loaded. It + // does not currently handle (or need to handle) the case where a project becomes unloaded. + this.setState({isProjectLoaded: true}); + } pushHistory (push) { // update URI to match mode const idPath = this.state.projectId ? `${this.state.projectId}/` : ''; @@ -440,6 +448,11 @@ class Preview extends React.Component { this.props.getTopLevelComments(this.state.projectId, this.props.comments.length, this.props.isAdmin, this.props.user && this.props.user.token); } + handleLoadMoreReplies (commentId, offset) { + this.props.getMoreReplies(this.state.projectId, commentId, offset, + this.props.isAdmin, this.props.user && this.props.user.token + ); + } handleLoveToggle () { if (!this.props.lovedLoaded) return; @@ -611,6 +624,7 @@ class Preview extends React.Component { isFullScreen={this.state.isFullScreen} isLoggedIn={this.props.isLoggedIn} isNewScratcher={this.props.isNewScratcher} + isProjectLoaded={this.state.isProjectLoaded} isRemixing={this.state.isRemixing} isScratcher={this.props.isScratcher} isShared={this.props.isShared} @@ -644,8 +658,10 @@ class Preview extends React.Component { onFavoriteClicked={this.handleFavoriteToggle} onGreenFlag={this.handleGreenFlag} onLoadMore={this.handleLoadMore} + onLoadMoreReplies={this.handleLoadMoreReplies} onLoveClicked={this.handleLoveToggle} onOpenAdminPanel={this.handleOpenAdminPanel} + onProjectLoaded={this.handleProjectLoaded} onRemix={this.handleRemix} onRemixing={this.handleIsRemixing} onReportClicked={this.handleReportClick} @@ -691,6 +707,7 @@ class Preview extends React.Component { onGreenFlag={this.handleGreenFlag} onLogOut={this.props.handleLogOut} onOpenRegistration={this.props.handleOpenRegistration} + onProjectLoaded={this.handleProjectLoaded} onRemixing={this.handleIsRemixing} onSetLanguage={this.handleSetLanguage} onShare={this.handleShare} @@ -736,6 +753,7 @@ Preview.propTypes = { getCuratedStudios: PropTypes.func.isRequired, getFavedStatus: PropTypes.func.isRequired, getLovedStatus: PropTypes.func.isRequired, + getMoreReplies: PropTypes.func.isRequired, getOriginalInfo: PropTypes.func.isRequired, getParentInfo: PropTypes.func.isRequired, getProjectInfo: PropTypes.func.isRequired, @@ -948,6 +966,9 @@ const mapDispatchToProps = dispatch => ({ getCommentById: (projectId, commentId, isAdmin, token) => { dispatch(previewActions.getCommentById(projectId, commentId, isAdmin, token)); }, + getMoreReplies: (projectId, commentId, offset, isAdmin, token) => { + dispatch(previewActions.getReplies(projectId, [commentId], offset, isAdmin, token)); + }, getFavedStatus: (id, username, token) => { dispatch(previewActions.getFavedStatus(id, username, token)); }, diff --git a/test/unit/redux/preview-test.js b/test/unit/redux/preview-test.js index 46c6f7d87..24a91ce84 100644 --- a/test/unit/redux/preview-test.js +++ b/test/unit/redux/preview-test.js @@ -59,9 +59,9 @@ tap.test('setComments', t => { // Initial value t.deepEqual(initialState.comments, []); - state = reducer(initialState, Preview.setComments([1, 2])); - state = reducer(state, Preview.setComments([3, 4])); - t.deepEqual(state.comments, [1, 2, 3, 4]); + state = reducer(initialState, Preview.setComments([{id: 1}, {id: 2}])); + state = reducer(state, Preview.setComments([{id: 3}, {id: 4}])); + t.deepEqual(state.comments, [{id: 1}, {id: 2}, {id: 3}, {id: 4}]); t.end(); }); @@ -80,6 +80,13 @@ const commentState = { } }; +tap.test('setComments, discards duplicates', t => { + state = reducer(commentState, Preview.setComments([{id: 'id1'}])); + // Does not increase the number of comments, still 3 + t.equal(state.comments.length, 3); + t.end(); +}); + tap.test('setCommentDeleted, top level comment', t => { state = reducer(commentState, Preview.setCommentDeleted('id2')); t.equal(state.comments[1].visibility, 'deleted'); @@ -128,3 +135,42 @@ tap.test('addNewComment, reply comment', t => { t.equal(state.replies.id1[2].id, 'new comment'); t.end(); }); + +tap.test('setReplies', t => { + // setReplies should append new replies + state = reducer(commentState, Preview.setReplies({ + id1: {id: 'id6'} + })); + t.equal(state.replies.id1[2].id, 'id6'); + t.equal(state.comments[0].moreRepliesToLoad, false); + + // setReplies should ignore duplicates, do the same as above again + t.equal(state.replies.id1.length, 3); + state = reducer(state, Preview.setReplies({id1: {id: 'id6'}})); + t.equal(state.replies.id1.length, 3); + + // setReplies can add replies to a comment that didn't have any + state = reducer(state, Preview.setReplies({ + id2: {id: 'id7'} + })); + t.equal(state.replies.id1.length, 3); + t.equal(state.replies.id2.length, 1); + t.equal(state.replies.id2[0].id, 'id7'); + t.equal(state.comments[0].moreRepliesToLoad, false); + t.equal(state.comments[1].moreRepliesToLoad, false); + + // Getting 20 (COMMENT_LIMIT) replies sets moreRepliesToLoad to true + state = reducer(state, Preview.setReplies({ + id3: (new Array(20)).map((_, i) => ({id: `id${i + 1}`})) + })); + t.equal(state.comments[0].moreRepliesToLoad, false); + t.equal(state.comments[1].moreRepliesToLoad, false); + t.equal(state.comments[2].moreRepliesToLoad, true); + + // Getting one more reply sets moreRepliesToLoad back to false + state = reducer(state, Preview.setReplies({ + id3: {id: 'id21'} + })); + t.equal(state.comments[2].moreRepliesToLoad, false); + t.end(); +});