From 39da42ee3f4a97eee727109b312338c8a35efedf Mon Sep 17 00:00:00 2001
From: ayesha waris <73840786+ayeshoali@users.noreply.github.com>
Date: Wed, 22 Mar 2023 17:19:15 +0500
Subject: [PATCH] fix: fix post height and remove overstate from author label
(#472)
* fix: fixed post length according to figma
* fix: remove hoverstate from author label and author icon
* style: adds tooltip in hovercard for endorsemment icon
* test: fix test cases
* test: adds test cases for descreaased coverage
* refactor: updated api call to a method for reusability
* refactor: changed test descriptions
---
src/discussions/common/AuthorLabel.jsx | 76 +++++++++----------
src/discussions/common/AuthorLabel.test.jsx | 7 +-
src/discussions/common/HoverCard.jsx | 39 ++++++----
.../post-comments/PostCommentsView.test.jsx | 24 +++++-
src/discussions/posts/post/Post.jsx | 17 ++---
src/discussions/posts/post/PostFooter.jsx | 13 ++--
.../posts/post/PostFooter.test.jsx | 38 ++++++----
7 files changed, 124 insertions(+), 90 deletions(-)
diff --git a/src/discussions/common/AuthorLabel.jsx b/src/discussions/common/AuthorLabel.jsx
index bf149941..37b674cb 100644
--- a/src/discussions/common/AuthorLabel.jsx
+++ b/src/discussions/common/AuthorLabel.jsx
@@ -44,34 +44,31 @@ function AuthorLabel({
const isRetiredUser = author ? author.startsWith('retired__user') : false;
const showTextPrimary = !authorLabelMessage && !isRetiredUser && !alert;
-
const className = classNames('d-flex align-items-center', { 'mb-0.5': !postOrComment }, labelColor);
const showUserNameAsLink = useShowLearnersTab()
&& linkToProfile && author && author !== intl.formatMessage(messages.anonymous);
+ const authorName = (
+
+ {isRetiredUser ? '[Deactivated]' : author}
+
+ );
const labelContents = (
-
)}
-
-
+ >
);
return showUserNameAsLink
? (
-
+
+
+ {!alert && authorName}
+
{labelContents}
-
+
)
- : <>{labelContents}>;
+ : {authorName}{labelContents}
;
}
AuthorLabel.propTypes = {
diff --git a/src/discussions/common/AuthorLabel.test.jsx b/src/discussions/common/AuthorLabel.test.jsx
index 57fea13f..b74c59aa 100644
--- a/src/discussions/common/AuthorLabel.test.jsx
+++ b/src/discussions/common/AuthorLabel.test.jsx
@@ -93,12 +93,13 @@ describe('Author label', () => {
async () => {
renderComponent(author, authorLabel, linkToProfile, labelColor);
const authorElement = container.querySelector('[role=heading]');
- const labelElement = authorElement.parentNode.lastChild;
+ const labelParentNode = authorElement.parentNode.parentNode;
+ const labelElement = labelParentNode.lastChild.lastChild;
const label = ['TA', 'Staff'].includes(labelElement.textContent) && labelElement.textContent;
if (linkToProfile) {
- expect(authorElement.parentNode).toHaveClass(labelColor);
- expect(authorElement.parentNode.lastChild).toHaveTextContent(label);
+ expect(labelParentNode).toHaveClass(labelColor);
+ expect(labelElement).toHaveTextContent(label);
} else {
expect(authorElement.parentNode.lastChild).not.toHaveTextContent(label, { exact: true });
expect(authorElement.parentNode).not.toHaveClass(labelColor, { exact: true });
diff --git a/src/discussions/common/HoverCard.jsx b/src/discussions/common/HoverCard.jsx
index 41c7b3af..cd8ae29f 100644
--- a/src/discussions/common/HoverCard.jsx
+++ b/src/discussions/common/HoverCard.jsx
@@ -3,8 +3,10 @@ import PropTypes from 'prop-types';
import classNames from 'classnames';
-import { injectIntl } from '@edx/frontend-platform/i18n';
-import { Button, Icon, IconButton } from '@edx/paragon';
+import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
+import {
+ Button, Icon, IconButton, OverlayTrigger, Tooltip,
+} from '@edx/paragon';
import {
StarFilled, StarOutline, ThumbUpFilled, ThumbUpOutline,
@@ -16,6 +18,7 @@ import ActionsDropdown from './ActionsDropdown';
import { DiscussionContext } from './context';
function HoverCard({
+ intl,
commentOrPost,
actionHandlers,
handleResponseCommentButton,
@@ -49,17 +52,26 @@ function HoverCard({
)}
{endorseIcons && (
- {
- const actionFunction = actionHandlers[endorseIcons.action];
- actionFunction();
- }}
- className={['endorse', 'unendorse'].includes(endorseIcons.id) ? 'text-dark-500' : 'text-success-500'}
- size="sm"
- alt="Endorse"
- />
+
+ {intl.formatMessage(endorseIcons.label)}
+
+ )}
+ trigger={['hover', 'focus']}
+ >
+ {
+ const actionFunction = actionHandlers[endorseIcons.action];
+ actionFunction();
+ }}
+ className={['endorse', 'unendorse'].includes(endorseIcons.id) ? 'text-dark-500' : 'text-success-500'}
+ size="sm"
+ alt="Endorse"
+ />
+
)}
@@ -98,6 +110,7 @@ function HoverCard({
}
HoverCard.propTypes = {
+ intl: intlShape.isRequired,
commentOrPost: PropTypes.oneOfType([commentShape, postShape]).isRequired,
actionHandlers: PropTypes.objectOf(PropTypes.func).isRequired,
handleResponseCommentButton: PropTypes.func.isRequired,
diff --git a/src/discussions/post-comments/PostCommentsView.test.jsx b/src/discussions/post-comments/PostCommentsView.test.jsx
index 20800400..a8b194d8 100644
--- a/src/discussions/post-comments/PostCommentsView.test.jsx
+++ b/src/discussions/post-comments/PostCommentsView.test.jsx
@@ -97,9 +97,9 @@ function mockAxiosReturnPagedCommentsResponses() {
}
}
-async function getThreadAPIResponse(threadId, topicId) {
+async function getThreadAPIResponse(attr = null) {
axiosMock.onGet(`${threadsApiUrl}${discussionPostId}/`)
- .reply(200, Factory.build('thread', { id: threadId, topic_id: topicId }));
+ .reply(200, Factory.build('thread', attr));
await executeThunk(fetchThread(discussionPostId), store.dispatch, store.getState);
}
@@ -152,14 +152,14 @@ describe('PostView', () => {
});
it('should show Topic Info for non-courseware topics', async () => {
- await getThreadAPIResponse('thread-1', 'non-courseware-topic-1');
+ await getThreadAPIResponse({ id: 'thread-1', topic_id: 'non-courseware-topic-1' });
renderComponent(discussionPostId);
expect(await screen.findByText('Related to')).toBeInTheDocument();
expect(await screen.findByText('non-courseware-topic 1')).toBeInTheDocument();
});
it('should show Topic Info for courseware topics with category', async () => {
- await getThreadAPIResponse('thread-2', 'courseware-topic-2');
+ await getThreadAPIResponse({ id: 'thread-2', topic_id: 'courseware-topic-2' });
renderComponent('thread-2');
expect(await screen.findByText('Related to')).toBeInTheDocument();
@@ -233,6 +233,22 @@ describe('ThreadView', () => {
renderComponent(discussionPostId);
const comment = await waitFor(() => screen.findByTestId('comment-comment-1'));
expect(within(comment).queryByTestId('comment-1')).toBeInTheDocument();
+ it('should not show post footer', async () => {
+ Factory.resetAll();
+ await getThreadAPIResponse({
+ vote_count: 0, following: false, closed: false, group_id: null,
+ });
+ renderComponent(discussionPostId);
+ expect(screen.queryByTestId('post-footer')).not.toBeInTheDocument();
+ });
+
+ it('should show post footer', async () => {
+ Factory.resetAll();
+ await getThreadAPIResponse({
+ vote_count: 4, following: true, closed: false, group_id: null,
+ });
+ renderComponent(discussionPostId);
+ expect(screen.queryByTestId('post-footer')).toBeInTheDocument();
});
it('should show and hide the editor', async () => {
diff --git a/src/discussions/posts/post/Post.jsx b/src/discussions/posts/post/Post.jsx
index 0bfad8c5..a4556b09 100644
--- a/src/discussions/posts/post/Post.jsx
+++ b/src/discussions/posts/post/Post.jsx
@@ -15,7 +15,7 @@ import { selectorForUnitSubsection, selectTopicContext } from '../../../data/sel
import { AlertBanner, Confirmation } from '../../common';
import { DiscussionContext } from '../../common/context';
import HoverCard from '../../common/HoverCard';
-import { selectModerationSettings } from '../../data/selectors';
+import { selectModerationSettings, selectUserHasModerationPrivileges } from '../../data/selectors';
import { selectTopic } from '../../topics/data/selectors';
import { removeThread, updateExistingThread } from '../data/thunks';
import ClosePostReasonModal from './ClosePostReasonModal';
@@ -26,7 +26,6 @@ import { postShape } from './proptypes';
function Post({
post,
- preview,
intl,
handleAddResponseButton,
}) {
@@ -42,6 +41,9 @@ function Post({
const [isDeleting, showDeleteConfirmation, hideDeleteConfirmation] = useToggle(false);
const [isReporting, showReportConfirmation, hideReportConfirmation] = useToggle(false);
const [isClosing, showClosePostModal, hideClosePostModal] = useToggle(false);
+ const userHasModerationPrivileges = useSelector(selectUserHasModerationPrivileges);
+ const displayPostFooter = post.following || post.voteCount || post.closed
+ || (post.groupId && userHasModerationPrivileges);
const handleAbusedFlag = useCallback(() => {
if (post.abuseFlagged) {
@@ -147,8 +149,8 @@ function Post({
{(topicContext || topic) && (
{intl.formatMessage(messages.relatedTo)}{' '}
@@ -170,7 +172,7 @@ function Post({
)}
-
+ {displayPostFooter && }
+
{post.voteCount !== 0 && (
-
+
,
);
@@ -64,23 +64,11 @@ describe('PostFooter', () => {
});
});
- it("doesn't have 'new' badge when there are 0 new comments", () => {
- renderComponent({ ...mockPost, unreadCommentCount: 0 });
- expect(screen.queryByText('2 New')).toBeFalsy();
- expect(screen.queryByText('0 New')).toBeFalsy();
- });
-
- it("doesn't has 'new' badge when the new-unread item is the post itself", () => {
- // commentCount === 1 means it's just the post without any further comments
- renderComponent({ ...mockPost, unreadCommentCount: 1, commentCount: 1 });
- expect(screen.queryByText('1 New')).toBeFalsy();
- });
-
it('has the cohort icon only when group information is present', () => {
renderComponent(mockPost);
expect(screen.queryByTestId('cohort-icon')).toBeFalsy();
- renderComponent({ ...mockPost, groupId: 5, groupName: 'Test Cohort' });
+ renderComponent({ ...mockPost, groupId: 5, groupName: 'Test Cohort' }, true);
expect(screen.getByTestId('cohort-icon')).toBeTruthy();
});
@@ -104,4 +92,24 @@ describe('PostFooter', () => {
renderComponent({ ...mockPost, following: false });
expect(screen.queryByRole('button', { name: /follow/i })).not.toBeInTheDocument();
});
+
+ it('tests like button when voteCount is zero', async () => {
+ renderComponent({ ...mockPost, voteCount: 0 });
+ expect(screen.queryByRole('button', { name: /like/i })).not.toBeInTheDocument();
+ });
+
+ it('tests like button when voteCount is not zero', async () => {
+ renderComponent({ ...mockPost, voted: true, voteCount: 4 });
+ const likeButton = screen.getByRole('button', { name: /like/i });
+ await act(async () => {
+ fireEvent.mouseEnter(likeButton);
+ });
+
+ expect(screen.getByRole('tooltip')).toHaveTextContent(/unlike/i);
+ await act(async () => {
+ fireEvent.click(likeButton);
+ });
+ // clicking on the button triggers thread update.
+ expect(store.getState().threads.status === RequestStatus.IN_PROGRESS).toBeTruthy();
+ });
});