From 8a73f23cb03ea6321cb5f09af075071230e006b6 Mon Sep 17 00:00:00 2001
From: Awais Ansari <79941147+awais-ansari@users.noreply.github.com>
Date: Wed, 31 Aug 2022 18:29:21 +0500
Subject: [PATCH] fix: small UI issue and post sorting (#274)

* fix: filter pinned, unpinned posts and render separately

* fix: UI updates for INF-421 and INF-417

* fix: pin icon moves to left for non privilege user

* fix: posts list accessibility issue
---
 src/components/SearchInfo.jsx                 |  2 +-
 src/discussions/learners/LearnerPostsView.jsx | 37 +++++++++---------
 src/discussions/learners/LearnersView.jsx     | 16 ++++++--
 .../learners/learner/LearnerCard.jsx          |  2 +-
 .../learners/learner/LearnerFilterBar.jsx     |  2 +-
 src/discussions/posts/PostsList.jsx           | 39 +++++++++----------
 src/discussions/posts/PostsView.jsx           |  1 +
 .../posts/post-actions-bar/PostActionsBar.jsx |  3 +-
 .../posts/post-filter-bar/PostFilterBar.jsx   |  2 +-
 src/discussions/posts/post/PostLink.jsx       | 19 ++++++---
 src/discussions/utils.js                      | 15 +++++++
 src/index.scss                                |  6 ++-
 12 files changed, 90 insertions(+), 54 deletions(-)

diff --git a/src/components/SearchInfo.jsx b/src/components/SearchInfo.jsx
index 6c2e061f..fc766cbd 100644
--- a/src/components/SearchInfo.jsx
+++ b/src/components/SearchInfo.jsx
@@ -16,7 +16,7 @@ function SearchInfo({
   onClear,
 }) {
   return (
-    <div className="d-flex flex-row border-bottom">
+    <div className="d-flex flex-row border-bottom border-light-400">
       <Icon src={Search} className="justify-content-start ml-3.5 mr-2 mb-2 mt-2.5" />
       <Button variant="" size="inline">
         {
diff --git a/src/discussions/learners/LearnerPostsView.jsx b/src/discussions/learners/LearnerPostsView.jsx
index a2732d63..e1368f7f 100644
--- a/src/discussions/learners/LearnerPostsView.jsx
+++ b/src/discussions/learners/LearnerPostsView.jsx
@@ -1,4 +1,6 @@
-import React, { useContext, useEffect } from 'react';
+import React, {
+  useCallback, useContext, useEffect, useMemo,
+} from 'react';
 
 import capitalize from 'lodash/capitalize';
 import { useDispatch, useSelector } from 'react-redux';
@@ -19,7 +21,7 @@ import {
 } from '../posts/data/selectors';
 import NoResults from '../posts/NoResults';
 import { PostLink } from '../posts/post';
-import { discussionsPath } from '../utils';
+import { discussionsPath, filterPosts } from '../utils';
 import { fetchUserPosts } from './data/thunks';
 import messages from './messages';
 
@@ -44,22 +46,20 @@ function LearnerPostsView({ intl }) {
   );
 
   const checkIsSelected = (id) => window.location.pathname.includes(id);
+  const pinnedPosts = useMemo(() => filterPosts(posts, 'pinned'), [posts]);
+  const unpinnedPosts = useMemo(() => filterPosts(posts, 'unpinned'), [posts]);
 
-  let lastPinnedIdx = null;
-  const postInstances = posts?.map((post, idx) => {
-    if (post.pinned && lastPinnedIdx !== false) {
-      lastPinnedIdx = idx;
-    } else if (lastPinnedIdx != null && lastPinnedIdx !== false) {
-      lastPinnedIdx = false;
-      // Add a spacing after the group of pinned posts
-      return (
-        <React.Fragment key={post.id}>
-          <PostLink post={post} key={post.id} isSelected={checkIsSelected} learnerTab showDivider />
-        </React.Fragment>
-      );
-    }
-    return (<PostLink post={post} key={post.id} isSelected={checkIsSelected} learnerTab />);
-  });
+  const postInstances = useCallback((sortedPosts) => (
+    sortedPosts.map((post, idx) => (
+      <PostLink
+        post={post}
+        key={post.id}
+        isSelected={checkIsSelected}
+        idx={idx}
+        showDivider={(sortedPosts.length - 1) !== idx}
+      />
+    ))
+  ), []);
 
   return (
     <div className="discussion-posts d-flex flex-column">
@@ -79,7 +79,8 @@ function LearnerPostsView({ intl }) {
       </div>
       <div className="bg-light-400 border border-light-300" />
       <div className="list-group list-group-flush">
-        {postInstances}
+        {postInstances(pinnedPosts)}
+        {postInstances(unpinnedPosts)}
         {loadingStatus !== RequestStatus.IN_PROGRESS && posts?.length === 0 && <NoResults />}
         {loadingStatus === RequestStatus.IN_PROGRESS ? (
           <div className="d-flex justify-content-center p-4">
diff --git a/src/discussions/learners/LearnersView.jsx b/src/discussions/learners/LearnersView.jsx
index 067f8711..8340be19 100644
--- a/src/discussions/learners/LearnersView.jsx
+++ b/src/discussions/learners/LearnersView.jsx
@@ -57,10 +57,18 @@ function LearnersView({ intl }) {
   };
 
   return (
-    <div className="d-flex flex-column border-right border-light-300 h-100">
-      { !usernameSearch && <LearnerFilterBar /> }
-      { usernameSearch && <SearchInfo text={usernameSearch} count={learners.length} loadingStatus={loadingStatus} onClear={() => dispatch(setUsernameSearch(''))} /> }
-      <div className="list-group list-group-flush learner flex-fill">
+    <div className="d-flex flex-column border-right border-light-400">
+      {!usernameSearch && <LearnerFilterBar /> }
+      <div className="border-bottom border-light-400" />
+      {usernameSearch && (
+        <SearchInfo
+          text={usernameSearch}
+          count={learners.length}
+          loadingStatus={loadingStatus}
+          onClear={() => dispatch(setUsernameSearch(''))}
+        />
+      )}
+      <div className="list-group list-group-flush learner" role="list">
         {courseConfigLoadingStatus === RequestStatus.SUCCESSFUL && !learnersTabEnabled && (
         <Redirect
           to={{
diff --git a/src/discussions/learners/learner/LearnerCard.jsx b/src/discussions/learners/learner/LearnerCard.jsx
index ee34f47d..b13a52de 100644
--- a/src/discussions/learners/learner/LearnerCard.jsx
+++ b/src/discussions/learners/learner/LearnerCard.jsx
@@ -25,7 +25,7 @@ function LearnerCard({
 
   return (
     <Link
-      className="discussion-post list-group-item list-group-item-action p-0 text-decoration-none text-gray-900 mw-100"
+      className="discussion-post p-0 text-decoration-none text-gray-900 border-bottom border-light-400"
       to={linkUrl}
     >
       <div
diff --git a/src/discussions/learners/learner/LearnerFilterBar.jsx b/src/discussions/learners/learner/LearnerFilterBar.jsx
index 856bac9a..f2451e58 100644
--- a/src/discussions/learners/learner/LearnerFilterBar.jsx
+++ b/src/discussions/learners/learner/LearnerFilterBar.jsx
@@ -65,7 +65,7 @@ function LearnerFilterBar({
     <Collapsible.Advanced
       open={isOpen}
       onToggle={() => setOpen(!isOpen)}
-      className="collapsible-card-lg border-right-0"
+      className="filter-bar collapsible-card-lg border-0"
     >
       <Collapsible.Trigger className="collapsible-trigger border-0">
         <span className="text-primary-700 pr-4">
diff --git a/src/discussions/posts/PostsList.jsx b/src/discussions/posts/PostsList.jsx
index 134557ae..34cf3515 100644
--- a/src/discussions/posts/PostsList.jsx
+++ b/src/discussions/posts/PostsList.jsx
@@ -1,7 +1,8 @@
-import React, { useContext, useEffect } from 'react';
+import React, {
+  useCallback, useContext, useEffect, useMemo,
+} from 'react';
 import PropTypes from 'prop-types';
 
-import uniqBy from 'lodash/uniqBy';
 import { useDispatch, useSelector } from 'react-redux';
 
 import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
@@ -12,6 +13,7 @@ import { RequestStatus } from '../../data/constants';
 import { DiscussionContext } from '../common/context';
 import { selectconfigLoadingStatus, selectUserHasModerationPrivileges, selectUserIsStaff } from '../data/selectors';
 import messages from '../messages';
+import { filterPosts } from '../utils';
 import {
   selectThreadFilters, selectThreadNextPage, selectThreadSorting, threadsLoadingStatus,
 } from './data/selectors';
@@ -53,28 +55,25 @@ function PostsList({ posts, topics, intl }) {
   }, [courseId, orderBy, filters, page, JSON.stringify(topics), configStatus]);
 
   const checkIsSelected = (id) => window.location.pathname.includes(id);
+  const pinnedPosts = useMemo(() => filterPosts(posts, 'pinned'), [posts]);
+  const unpinnedPosts = useMemo(() => filterPosts(posts, 'unpinned'), [posts]);
 
-  let lastPinnedIdx = null;
-  const sortedPosts = uniqBy(posts, 'id').sort((a, b) => b.pinned - a.pinned);
-
-  const postInstances = sortedPosts.map((post, idx) => {
-    if (post.pinned && lastPinnedIdx !== false) {
-      lastPinnedIdx = idx;
-    } else if (lastPinnedIdx != null && lastPinnedIdx !== false) {
-      lastPinnedIdx = false;
-      // Add a spacing after the group of pinned posts
-      return (
-        <React.Fragment key={post.id}>
-          <PostLink post={post} key={post.id} isSelected={checkIsSelected} showDivider idx={idx} />
-        </React.Fragment>
-      );
-    }
-    return (<PostLink post={post} key={post.id} isSelected={checkIsSelected} idx={idx} />);
-  });
+  const postInstances = useCallback((sortedPosts) => (
+    sortedPosts.map((post, idx) => (
+      <PostLink
+        post={post}
+        key={post.id}
+        isSelected={checkIsSelected}
+        idx={idx}
+        showDivider={(sortedPosts.length - 1) !== idx}
+      />
+    ))
+  ), []);
 
   return (
     <>
-      {postInstances}
+      {postInstances(pinnedPosts)}
+      {postInstances(unpinnedPosts)}
       {posts?.length === 0 && loadingStatus === RequestStatus.SUCCESSFUL && <NoResults />}
       {loadingStatus === RequestStatus.IN_PROGRESS ? (
         <div className="d-flex justify-content-center p-4">
diff --git a/src/discussions/posts/PostsView.jsx b/src/discussions/posts/PostsView.jsx
index 094ed4e4..d55739ad 100644
--- a/src/discussions/posts/PostsView.jsx
+++ b/src/discussions/posts/PostsView.jsx
@@ -80,6 +80,7 @@ function PostsView() {
         searchString && <SearchInfo count={resultsFound} text={searchString} loadingStatus={loadingStatus} onClear={() => dispatch(setSearchQuery(''))} />
       }
       <PostFilterBar filterSelfPosts={showOwnPosts} />
+      <div className="border-bottom border-light-400" />
       <div className="list-group list-group-flush flex-fill" role="list" onKeyDown={e => handleKeyDown(e)}>
         {postsListComponent}
       </div>
diff --git a/src/discussions/posts/post-actions-bar/PostActionsBar.jsx b/src/discussions/posts/post-actions-bar/PostActionsBar.jsx
index 3d40ce0e..10d0548a 100644
--- a/src/discussions/posts/post-actions-bar/PostActionsBar.jsx
+++ b/src/discussions/posts/post-actions-bar/PostActionsBar.jsx
@@ -29,6 +29,7 @@ function PostActionsBar({
       {!inContext && (
         <>
           <Search />
+          <div className="border-right border-light-400 mx-3" />
         </>
       )}
       {inContext && (
@@ -39,7 +40,7 @@ function PostActionsBar({
 
       <Button
         variant={inContext ? 'plain' : 'brand'}
-        className="ml-2 my-0"
+        className="my-0"
         onClick={() => dispatch(showPostEditor())}
         size="sm"
       >
diff --git a/src/discussions/posts/post-filter-bar/PostFilterBar.jsx b/src/discussions/posts/post-filter-bar/PostFilterBar.jsx
index 797d9169..71ad31a6 100644
--- a/src/discussions/posts/post-filter-bar/PostFilterBar.jsx
+++ b/src/discussions/posts/post-filter-bar/PostFilterBar.jsx
@@ -115,7 +115,7 @@ function PostFilterBar({
     <Collapsible.Advanced
       open={isOpen}
       onToggle={() => setOpen(!isOpen)}
-      className="collapsible-card-lg border-right-0"
+      className="filter-bar collapsible-card-lg border-0"
     >
       <Collapsible.Trigger className="collapsible-trigger border-0">
         <span className="text-primary-700 pr-4">
diff --git a/src/discussions/posts/post/PostLink.jsx b/src/discussions/posts/post/PostLink.jsx
index 882907b5..9dd87fb7 100644
--- a/src/discussions/posts/post/PostLink.jsx
+++ b/src/discussions/posts/post/PostLink.jsx
@@ -47,13 +47,16 @@ function PostLink({
   const showAnsweredBadge = post.hasEndorsed && post.type === ThreadType.QUESTION;
   const authorLabelColor = AvatarOutlineAndLabelColors[post.authorLabel];
   const postReported = post.abuseFlagged || post.abuseFlaggedCount;
-  const canSeeReportedBadge = (userHasModerationPrivileges || userIsGroupTa);
+  const canSeeReportedBadge = postReported && (userHasModerationPrivileges || userIsGroupTa);
 
   return (
     <>
-      {showDivider && <div className="p-1 bg-light-400" />}
       <Link
-        className="discussion-post list-group-item list-group-item-action p-0 text-decoration-none text-gray-900"
+        className={
+          classNames('discussion-post p-0 text-decoration-none text-gray-900', {
+            'border-bottom border-light-400': showDivider,
+          })
+        }
         to={linkUrl}
         onClick={() => isSelected(post.id)}
         style={{ lineHeight: '21px' }}
@@ -91,7 +94,7 @@ function PostLink({
                 </Badge>
                 )}
 
-                {postReported && canSeeReportedBadge && (
+                {canSeeReportedBadge && (
                 <Badge
                   variant="danger"
                   data-testid="reported-post"
@@ -103,7 +106,10 @@ function PostLink({
                 )}
 
                 {post.pinned && (
-                <Icon src={PushPin} className={`icon-size ${postReported || showAnsweredBadge ? 'ml-2' : 'ml-auto'}`} />
+                <Icon
+                  src={PushPin}
+                  className={`icon-size ${canSeeReportedBadge || showAnsweredBadge ? 'ml-2' : 'ml-auto'}`}
+                />
                 )}
               </div>
             </div>
@@ -126,6 +132,7 @@ function PostLink({
             </div>
           </div>
         </div>
+        {!showDivider && post.pinned && <div className="pt-1 bg-light-500 border-top border-light-700" />}
       </Link>
     </>
   );
@@ -142,7 +149,7 @@ PostLink.propTypes = {
 
 PostLink.defaultProps = {
   learnerTab: false,
-  showDivider: false,
+  showDivider: true,
   idx: -1,
 };
 
diff --git a/src/discussions/utils.js b/src/discussions/utils.js
index bafca548..28f4b964 100644
--- a/src/discussions/utils.js
+++ b/src/discussions/utils.js
@@ -1,5 +1,6 @@
 /* eslint-disable import/prefer-default-export */
 import { getIn } from 'formik';
+import { orderBy, uniqBy } from 'lodash';
 import { generatePath, useRouteMatch } from 'react-router';
 
 import { getConfig } from '@edx/frontend-platform';
@@ -238,3 +239,17 @@ export const isPostPreviewAvailable = (htmlNode) => {
   }
   return true;
 };
+
+/**
+ * Helper function to filter posts
+ * @param {array} posts arrays of posts
+ * @param {string} filterBy name of post object attribute. un will use for reverse
+ *  condition. like pinned attribute for pinned post and unpinned for non pinned posts.
+ * @param {string} sortBy name of post object attribute
+ * @param {string} order order of the sorting list
+ */
+export const filterPosts = (posts, filterBy, sortBy = 'createdAt', order = 'desc') => orderBy(
+  uniqBy(posts, 'id').filter(
+    post => (filterBy.startsWith('un') ? !post[filterBy.slice(2)] : post[filterBy]),
+  ), [sortBy], [order],
+);
diff --git a/src/index.scss b/src/index.scss
index e8745223..62f69192 100755
--- a/src/index.scss
+++ b/src/index.scss
@@ -160,4 +160,8 @@ header {
 
 .pointer-cursor-hover :hover{
   cursor: pointer;
-}
\ No newline at end of file
+}
+
+.filter-bar:focus-visible, .filter-bar:focus {
+  outline: none;
+}