From d3adb8b3e77300423a0382556ef007d412a5c2a3 Mon Sep 17 00:00:00 2001 From: Awais Ansari <79941147+awais-ansari@users.noreply.github.com> Date: Wed, 27 Jul 2022 15:32:34 +0500 Subject: [PATCH] fix: response and comment UI according to Figma (#220) * fix: response and comment UI according to Figma * test: add test cases for endorse alert banner --- src/discussions/comments/CommentsView.jsx | 4 +- .../comments/comment-icons/CommentIcons.jsx | 11 +- src/discussions/comments/comment/Comment.jsx | 123 +++++++++--------- .../comments/comment/CommentHeader.jsx | 23 +++- src/discussions/comments/comment/Reply.jsx | 40 +++--- src/discussions/common/ActionsDropdown.jsx | 19 ++- src/discussions/common/AlertBanner.jsx | 48 +------ src/discussions/common/AlertBanner.test.jsx | 27 +--- src/discussions/common/AuthorLabel.jsx | 9 +- .../common/EndorsedAlertBanner.jsx | 66 ++++++++++ .../common/EndorsedAlertBanner.test.jsx | 87 +++++++++++++ src/discussions/common/index.js | 1 + src/discussions/data/hooks.js | 15 ++- src/discussions/posts/post/Post.jsx | 2 +- src/discussions/posts/post/PostHeader.jsx | 5 +- src/index.scss | 8 +- 16 files changed, 316 insertions(+), 172 deletions(-) create mode 100644 src/discussions/common/EndorsedAlertBanner.jsx create mode 100644 src/discussions/common/EndorsedAlertBanner.test.jsx diff --git a/src/discussions/comments/CommentsView.jsx b/src/discussions/comments/CommentsView.jsx index c9f60121..9f912520 100644 --- a/src/discussions/comments/CommentsView.jsx +++ b/src/discussions/comments/CommentsView.jsx @@ -67,12 +67,12 @@ function DiscussionCommentsView({ } = usePostComments(postId, endorsed); return ( <> -
+
{endorsed === EndorsementStatus.ENDORSED ? intl.formatMessage(messages.endorsedResponseCount, { num: comments.length }) : intl.formatMessage(messages.responseCount, { num: comments.length })}
-
+
{comments.map(comment => ( ))} diff --git a/src/discussions/comments/comment-icons/CommentIcons.jsx b/src/discussions/comments/comment-icons/CommentIcons.jsx index efabc22f..1a87d284 100644 --- a/src/discussions/comments/comment-icons/CommentIcons.jsx +++ b/src/discussions/comments/comment-icons/CommentIcons.jsx @@ -4,16 +4,18 @@ import PropTypes from 'prop-types'; import { useDispatch } from 'react-redux'; import * as timeago from 'timeago.js'; -import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { injectIntl } from '@edx/frontend-platform/i18n'; +import timeLocale from '../../common/time-locale'; import LikeButton from '../../posts/post/LikeButton'; import { editComment } from '../data/thunks'; function CommentIcons({ comment, - intl, }) { const dispatch = useDispatch(); + timeago.register('time-locale', timeLocale); + const handleLike = () => dispatch(editComment(comment.id, { voted: !comment.voted })); return (
@@ -22,8 +24,8 @@ function CommentIcons({ onClick={handleLike} voted={comment.voted} /> -
- {timeago.format(comment.createdAt, intl.locale)} +
+ {timeago.format(comment.createdAt, 'time-locale')}
); @@ -37,7 +39,6 @@ CommentIcons.propTypes = { voted: PropTypes.bool, createdAt: PropTypes.string, }).isRequired, - intl: intlShape.isRequired, }; export default injectIntl(CommentIcons); diff --git a/src/discussions/comments/comment/Comment.jsx b/src/discussions/comments/comment/Comment.jsx index 1c70b544..650d71d4 100644 --- a/src/discussions/comments/comment/Comment.jsx +++ b/src/discussions/comments/comment/Comment.jsx @@ -9,7 +9,7 @@ import { Button, useToggle } from '@edx/paragon'; import HTMLLoader from '../../../components/HTMLLoader'; import { ContentActions } from '../../../data/constants'; -import { AlertBanner, DeleteConfirmation } from '../../common'; +import { AlertBanner, DeleteConfirmation, EndorsedAlertBanner } from '../../common'; import { fetchThread } from '../../posts/data/thunks'; import CommentIcons from '../comment-icons/CommentIcons'; import { selectCommentCurrentPage, selectCommentHasMorePages, selectCommentResponses } from '../data/selectors'; @@ -35,12 +35,14 @@ function Comment({ const [isReplying, setReplying] = useState(false); const hasMorePages = useSelector(selectCommentHasMorePages(comment.id)); const currentPage = useSelector(selectCommentCurrentPage(comment.id)); + useEffect(() => { // If the comment has a parent comment, it won't have any children, so don't fetch them. if (hasChildren && !currentPage && showFullThread) { dispatch(fetchCommentResponses(comment.id, { page: 1 })); } }, [comment.id]); + const actionHandlers = { [ContentActions.EDIT_CONTENT]: () => setEditing(true), [ContentActions.ENDORSE]: async () => { @@ -50,79 +52,82 @@ function Comment({ [ContentActions.DELETE]: showDeleteConfirmation, [ContentActions.REPORT]: () => dispatch(editComment(comment.id, { flagged: !comment.abuseFlagged })), }; + const handleLoadMoreComments = () => ( dispatch(fetchCommentResponses(comment.id, { page: currentPage + 1 })) ); - const commentClasses = classNames('d-flex flex-column card', { 'my-3': showFullThread }); return ( -
- { - dispatch(removeComment(comment.id)); - hideDeleteConfirmation(); - }} - /> - -
- - {isEditing - ? ( - setEditing(false)} /> - ) - : } - dispatch(editComment(comment.id, { voted: !comment.voted }))} - createdAt={comment.createdAt} +
+
+ { + dispatch(removeComment(comment.id)); + hideDeleteConfirmation(); + }} /> -
{intl.formatMessage(messages.replies, { count: inlineReplies.length })}
-
- {/* Pass along intl since component used here is the one before it's injected with `injectIntl` */} - {inlineReplies.map(inlineReply => ( - - ))} -
- {hasMorePages && ( + +
+ + + {isEditing + ? ( + setEditing(false)} /> + ) + : } + dispatch(editComment(comment.id, { voted: !comment.voted }))} + createdAt={comment.createdAt} + /> +
{intl.formatMessage(messages.replies, { count: inlineReplies.length })}
+
+ {/* Pass along intl since component used here is the one before it's injected with `injectIntl` */} + {inlineReplies.map(inlineReply => ( + + ))} +
+ {hasMorePages && ( - )} - {!isNested && showFullThread - && ( - isReplying - ? ( - setReplying(false)} - /> - ) - : ( - - ) )} + {!isNested && showFullThread && ( + isReplying ? ( + setReplying(false)} + /> + ) : ( + + ) + )} +
); diff --git a/src/discussions/comments/comment/CommentHeader.jsx b/src/discussions/comments/comment/CommentHeader.jsx index d3388717..cada8c17 100644 --- a/src/discussions/comments/comment/CommentHeader.jsx +++ b/src/discussions/comments/comment/CommentHeader.jsx @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; +import classNames from 'classnames'; import { useSelector } from 'react-redux'; import { injectIntl } from '@edx/frontend-platform/i18n'; @@ -10,6 +11,7 @@ import { CheckCircle, Verified } from '@edx/paragon/icons'; import { AvatarOutlineAndLabelColors, ThreadType } from '../../../data/constants'; import { AuthorLabel } from '../../common'; import ActionsDropdown from '../../common/ActionsDropdown'; +import { useAlertBannerVisible } from '../../data/hooks'; import { selectAuthorAvatars } from '../../posts/data/selectors'; import { commentShape } from './proptypes'; @@ -20,20 +22,31 @@ function CommentHeader({ }) { const authorAvatars = useSelector(selectAuthorAvatars(comment.author)); const colorClass = AvatarOutlineAndLabelColors[comment.authorLabel]; + const hasAnyAlert = useAlertBannerVisible(comment); + return ( -
+
- {comment.endorsed && (postType === 'question' - ? - : )} + + {comment.endorsed && (postType === 'question' + ? + : )} + +
-
-
- + {hasAnyAlert && ( +
+
+ +
+
+ +
-
- -
-
+ )}
-
+
-
-
+
+
{isEditing ? setEditing(false)} /> - : } + : }
- {timeago.format(reply.createdAt, intl.locale)} + {timeago.format(reply.createdAt, 'time-locale')}
); diff --git a/src/discussions/common/ActionsDropdown.jsx b/src/discussions/common/ActionsDropdown.jsx index c0fa2ce6..4a03d843 100644 --- a/src/discussions/common/ActionsDropdown.jsx +++ b/src/discussions/common/ActionsDropdown.jsx @@ -33,16 +33,15 @@ function ActionsDropdown({ }; return ( <> - - setOpen(!isOpen)} - alt={intl.formatMessage(messages.actionsAlt)} - src={MoreHoriz} - iconAs={Icon} - disabled={disabled} - size="sm" - /> - + setOpen(!isOpen)} + alt={intl.formatMessage(messages.actionsAlt)} + src={MoreHoriz} + iconAs={Icon} + disabled={disabled} + size="sm" + ref={dropdownIconRef} + /> setOpen(false)} positionRef={dropdownIconRef} diff --git a/src/discussions/common/AlertBanner.jsx b/src/discussions/common/AlertBanner.jsx index fbcd4f18..f66a65d0 100644 --- a/src/discussions/common/AlertBanner.jsx +++ b/src/discussions/common/AlertBanner.jsx @@ -2,13 +2,11 @@ import React from 'react'; import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; -import * as timeago from 'timeago.js'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Alert } from '@edx/paragon'; -import { CheckCircle, Error, Verified } from '@edx/paragon/icons'; +import { Error } from '@edx/paragon/icons'; -import { ThreadType } from '../../data/constants'; import { commentShape } from '../comments/comment/proptypes'; import messages from '../comments/messages'; import { selectModerationSettings, selectUserIsPrivileged } from '../data/selectors'; @@ -18,50 +16,19 @@ import AuthorLabel from './AuthorLabel'; function AlertBanner({ intl, content, - postType, }) { - const isQuestion = postType === ThreadType.QUESTION; - const classes = isQuestion ? 'bg-success-500 text-white' : 'bg-dark-500 text-white'; - const iconClass = isQuestion ? CheckCircle : Verified; const userIsPrivileged = useSelector(selectUserIsPrivileged); const { reasonCodesEnabled } = useSelector(selectModerationSettings); + return ( <> - {content.endorsed && ( - -
- {intl.formatMessage( - isQuestion - ? messages.answer - : messages.endorsed, - )} - - - - {intl.formatMessage( - isQuestion - ? messages.answeredLabel - : messages.endorsedLabel, - )} - - - {timeago.format(content.endorsedAt, intl.locale)} - -
- - )} {content.abuseFlagged && ( - + {intl.formatMessage(messages.abuseFlaggedMessage)} )} {reasonCodesEnabled && userIsPrivileged && content.lastEdit?.reason && ( - +
{intl.formatMessage(messages.editedBy)} @@ -72,7 +39,7 @@ function AlertBanner({ )} {reasonCodesEnabled && content.closed && ( - +
{intl.formatMessage(messages.closedBy)} @@ -90,11 +57,6 @@ function AlertBanner({ AlertBanner.propTypes = { intl: intlShape.isRequired, content: PropTypes.oneOfType([commentShape.isRequired, postShape.isRequired]).isRequired, - postType: PropTypes.string, -}; - -AlertBanner.defaultProps = { - postType: null, }; export default injectIntl(AlertBanner); diff --git a/src/discussions/common/AlertBanner.test.jsx b/src/discussions/common/AlertBanner.test.jsx index 1ec499e8..1928d11a 100644 --- a/src/discussions/common/AlertBanner.test.jsx +++ b/src/discussions/common/AlertBanner.test.jsx @@ -22,14 +22,12 @@ function buildTestContent(type, buildParams) { function renderComponent( content, - postType, ) { render( , @@ -44,27 +42,6 @@ describe.each([ props: { abuseFlagged: true }, expectText: [messages.abuseFlaggedMessage.defaultMessage], }, - { - label: 'Staff endorsed comment in a question thread', - type: 'comment', - postType: ThreadType.QUESTION, - props: { endorsed: true, endorsedBy: 'test-user', endorsedByLabel: 'Staff' }, - expectText: [messages.answer.defaultMessage, messages.answeredLabel.defaultMessage, 'test-user', 'Staff'], - }, - { - label: 'TA endorsed comment in a question thread', - type: 'comment', - postType: ThreadType.QUESTION, - props: { endorsed: true, endorsedBy: 'test-user', endorsedByLabel: 'Community TA' }, - expectText: [messages.answer.defaultMessage, messages.answeredLabel.defaultMessage, 'test-user', 'TA'], - }, - { - label: 'endorsed comment in a discussion thread', - type: 'comment', - postType: ThreadType.DISCUSSION, - props: { endorsed: true, endorsedBy: 'test-user' }, - expectText: [messages.endorsed.defaultMessage, messages.endorsedLabel.defaultMessage, 'test-user'], - }, { label: 'flagged thread', type: 'thread', @@ -87,7 +64,7 @@ describe.each([ expectText: [messages.closedBy.defaultMessage, 'closing-user', 'test-close-reason'], }, ])('AlertBanner', ({ - label, type, postType, props, expectText, + label, type, props, expectText, }) => { beforeEach(async () => { initializeMockApp({ @@ -105,7 +82,7 @@ describe.each([ }, }); const content = buildTestContent(type, props); - renderComponent(content, postType); + renderComponent(content); }); it(`should show correct banner for a ${label}`, async () => { diff --git a/src/discussions/common/AuthorLabel.jsx b/src/discussions/common/AuthorLabel.jsx index 83826df3..06d9ee52 100644 --- a/src/discussions/common/AuthorLabel.jsx +++ b/src/discussions/common/AuthorLabel.jsx @@ -34,13 +34,14 @@ function AuthorLabel({ authorLabelMessage = intl.formatMessage(messages.authorLabelTA); } - const fontWeight = authorLabelMessage ? 'font-weight-500' : 'font-weight-normal text-primary-500'; const className = classNames('d-flex align-items-center', labelColor); const labelContents = (
@@ -57,7 +58,9 @@ function AuthorLabel({ )} {authorLabelMessage && ( {authorLabelMessage} diff --git a/src/discussions/common/EndorsedAlertBanner.jsx b/src/discussions/common/EndorsedAlertBanner.jsx new file mode 100644 index 00000000..789634a7 --- /dev/null +++ b/src/discussions/common/EndorsedAlertBanner.jsx @@ -0,0 +1,66 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import * as timeago from 'timeago.js'; + +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { Alert } from '@edx/paragon'; +import { CheckCircle, Verified } from '@edx/paragon/icons'; + +import { ThreadType } from '../../data/constants'; +import { commentShape } from '../comments/comment/proptypes'; +import messages from '../comments/messages'; +import AuthorLabel from './AuthorLabel'; + +function EndorsedAlertBanner({ + intl, + content, + postType, +}) { + const isQuestion = postType === ThreadType.QUESTION; + const classes = isQuestion ? 'bg-success-500 text-white' : 'bg-dark-500 text-white'; + const iconClass = isQuestion ? CheckCircle : Verified; + + return ( + content.endorsed && ( + +
+ {intl.formatMessage( + isQuestion + ? messages.answer + : messages.endorsed, + )} + + + + {intl.formatMessage( + isQuestion + ? messages.answeredLabel + : messages.endorsedLabel, + )} + + + {timeago.format(content.endorsedAt, intl.locale)} + +
+ + ) + ); +} + +EndorsedAlertBanner.propTypes = { + intl: intlShape.isRequired, + content: PropTypes.oneOfType([commentShape.isRequired]).isRequired, + postType: PropTypes.string, +}; + +EndorsedAlertBanner.defaultProps = { + postType: null, +}; + +export default injectIntl(EndorsedAlertBanner); diff --git a/src/discussions/common/EndorsedAlertBanner.test.jsx b/src/discussions/common/EndorsedAlertBanner.test.jsx new file mode 100644 index 00000000..4248249f --- /dev/null +++ b/src/discussions/common/EndorsedAlertBanner.test.jsx @@ -0,0 +1,87 @@ +import { render, screen } from '@testing-library/react'; +import { IntlProvider } from 'react-intl'; +import { Factory } from 'rosie'; + +import { camelCaseObject, initializeMockApp, snakeCaseObject } from '@edx/frontend-platform'; +import { AppProvider } from '@edx/frontend-platform/react'; + +import { ThreadType } from '../../data/constants'; +import { initializeStore } from '../../store'; +import messages from '../comments/messages'; +import EndorsedAlertBanner from './EndorsedAlertBanner'; + +import '../comments/data/__factories__'; +import '../posts/data/__factories__'; + +let store; + +function buildTestContent(type, buildParams) { + const buildParamsSnakeCase = snakeCaseObject(buildParams); + return camelCaseObject(Factory.build(type, { ...buildParamsSnakeCase }, null)); +} + +function renderComponent( + content, postType, +) { + render( + + + + + , + ); +} + +describe.each([ + { + label: 'Staff endorsed comment in a question thread', + type: 'comment', + postType: ThreadType.QUESTION, + props: { endorsed: true, endorsedBy: 'test-user', endorsedByLabel: 'Staff' }, + expectText: [messages.answer.defaultMessage, messages.answeredLabel.defaultMessage, 'test-user', 'Staff'], + }, + { + label: 'TA endorsed comment in a question thread', + type: 'comment', + postType: ThreadType.QUESTION, + props: { endorsed: true, endorsedBy: 'test-user', endorsedByLabel: 'Community TA' }, + expectText: [messages.answer.defaultMessage, messages.answeredLabel.defaultMessage, 'test-user', 'TA'], + }, + { + label: 'endorsed comment in a discussion thread', + type: 'comment', + postType: ThreadType.DISCUSSION, + props: { endorsed: true, endorsedBy: 'test-user' }, + expectText: [messages.endorsed.defaultMessage, messages.endorsedLabel.defaultMessage, 'test-user'], + }, +])('EndorsedAlertBanner', ({ + label, type, postType, props, expectText, +}) => { + beforeEach(async () => { + initializeMockApp({ + authenticatedUser: { + userId: 3, + username: 'abc123', + administrator: false, + roles: [], + }, + }); + store = initializeStore({ + config: { + userIsPrivileged: true, + reasonCodesEnabled: true, + }, + }); + const content = buildTestContent(type, props); + renderComponent(content, postType); + }); + + it(`should show correct banner for a ${label}`, async () => { + expectText.forEach(message => { + expect(screen.queryAllByText(message, { exact: false }).length).toBeGreaterThan(0); + }); + }); +}); diff --git a/src/discussions/common/index.js b/src/discussions/common/index.js index e80c85a7..4bee420d 100644 --- a/src/discussions/common/index.js +++ b/src/discussions/common/index.js @@ -2,3 +2,4 @@ export { default as ActionsDropdown } from './ActionsDropdown'; export { default as AlertBanner } from './AlertBanner'; export { default as AuthorLabel } from './AuthorLabel'; export { default as DeleteConfirmation } from './DeleteConfirmation'; +export { default as EndorsedAlertBanner } from './EndorsedAlertBanner'; diff --git a/src/discussions/data/hooks.js b/src/discussions/data/hooks.js index 443547c0..64039a81 100644 --- a/src/discussions/data/hooks.js +++ b/src/discussions/data/hooks.js @@ -13,7 +13,9 @@ import { clearRedirect } from '../posts/data'; import { selectTopics } from '../topics/data/selectors'; import { fetchCourseTopics } from '../topics/data/thunks'; import { discussionsPath, postMessageToParent } from '../utils'; -import { selectAreThreadsFiltered, selectPostThreadCount } from './selectors'; +import { + selectAreThreadsFiltered, selectModerationSettings, selectPostThreadCount, selectUserIsPrivileged, +} from './selectors'; import { fetchCourseConfig } from './thunks'; export function useTotalTopicThreadCount() { @@ -141,3 +143,14 @@ export function useContainerSizeForParent(refContainer) { }; }, [refContainer, resizeObserver, location]); } + +export const useAlertBannerVisible = (content) => { + const userIsPrivileged = useSelector(selectUserIsPrivileged); + const { reasonCodesEnabled } = useSelector(selectModerationSettings); + + return ( + (content.abuseFlagged) + || (reasonCodesEnabled && userIsPrivileged && content.lastEdit?.reason) + || (reasonCodesEnabled && content.closed) + ); +}; diff --git a/src/discussions/posts/post/Post.jsx b/src/discussions/posts/post/Post.jsx index e6830200..bc148d8c 100644 --- a/src/discussions/posts/post/Post.jsx +++ b/src/discussions/posts/post/Post.jsx @@ -70,7 +70,7 @@ function Post({ hideDeleteConfirmation(); }} /> - +
diff --git a/src/discussions/posts/post/PostHeader.jsx b/src/discussions/posts/post/PostHeader.jsx index b5378cb3..b5c7ddb5 100644 --- a/src/discussions/posts/post/PostHeader.jsx +++ b/src/discussions/posts/post/PostHeader.jsx @@ -10,6 +10,7 @@ import { Avatar, Badge, Icon } from '@edx/paragon'; import { Issue, Question } from '../../../components/icons'; import { AvatarOutlineAndLabelColors, ThreadType } from '../../../data/constants'; import { ActionsDropdown, AuthorLabel } from '../../common'; +import { useAlertBannerVisible } from '../../data/hooks'; import { selectAuthorAvatars } from '../data/selectors'; import messages from './messages'; import { postShape } from './proptypes'; @@ -89,8 +90,10 @@ function PostHeader({ }) { const showAnsweredBadge = preview && post.hasEndorsed && post.type === ThreadType.QUESTION; const authorLabelColor = AvatarOutlineAndLabelColors[post.authorLabel]; + const hasAnyAlert = useAlertBannerVisible(post); + return ( -
+
diff --git a/src/index.scss b/src/index.scss index 7231b402..54d200b0 100755 --- a/src/index.scss +++ b/src/index.scss @@ -63,6 +63,10 @@ $fa-font-path: "~font-awesome/fonts"; margin-right: 2px; } +.mb-0\.5 { + margin-bottom: 2px; +} + .mt-0\.5 { margin-top: 2px; } @@ -100,6 +104,6 @@ $fa-font-path: "~font-awesome/fonts"; } .avarat-img-position { -margin-top: 17px; -margin-left: 17px; + margin-top: 17px; + margin-left: 17px; }