Bugs and ux issues (#222)

* Fixing translations. * Fixing issues with public exercises. * Improving appearance of Group hierarchy box. * Fixing UI reload issues associated with language/localization changes. * Fixing bug in sorting table. Preventing members of groups which have publicStats === false from seeing other users in the table.
ReCodEx · Apr 28, 2018 · a8dd779 · a8dd779
1 parent b13a7bc
commit a8dd779
Show file tree

Hide file tree

Showing 13 changed files with 195 additions and 156 deletions.
diff --git a/src/components/Groups/GroupTree/GroupTree.js b/src/components/Groups/GroupTree/GroupTree.js
@@ -12,6 +12,13 @@ import { getLocalizedResourceName } from '../../../helpers/getLocalizedData';
 import { GroupIcon } from '../../icons';
 import withLinks from '../../../helpers/withLinks';
 
+const conditionalEmphasize = (content, condition) =>
+  condition
+    ? <strong>
+        {content}
+      </strong>
+    : content;
+
 class GroupTree extends Component {
   renderLoading = level =>
     <TreeView>
@@ -56,9 +63,10 @@ class GroupTree extends Component {
 
   renderChildGroups = (
     { all: allChildGroups, public: publicChildGroups },
-    visibleGroupsMap
+    visibleGroupsMap,
+    level
   ) => {
-    const { level = 0, isOpen = false, groups, intl: { locale } } = this.props;
+    const { isOpen = false, groups, intl: { locale } } = this.props;
     return allChildGroups
       .filter(id => visibleGroupsMap[id])
       .sort((id1, id2) => {
@@ -89,15 +97,20 @@ class GroupTree extends Component {
       isPublic = false,
       groups,
       currentGroupId = null,
-      visibleGroupsMap = null
+      visibleGroupsMap = null,
+      ancestralPath = null
     } = this.props;
 
-    const group = groups.get(id);
+    const onAncestralPath = ancestralPath && ancestralPath.length > 0;
+    const group = onAncestralPath
+      ? groups.get(ancestralPath[0])
+      : groups.get(id);
     if (!group || !isReady(group)) {
       return this.renderLoading(level);
     }
 
     const {
+      id: groupId,
       name,
       localizedTexts,
       canView,
@@ -113,33 +126,47 @@ class GroupTree extends Component {
 
     return (
       <TreeView>
-        {level !== 0 &&
-          <TreeViewItem
-            title={
-              <GroupsName
-                id={id}
-                name={name}
-                localizedTexts={localizedTexts}
-                noLink
-              />
-            }
-            id={id}
-            level={level}
-            admins={primaryAdminsIds}
-            organizational={organizational}
-            isPublic={isPublic}
-            isOpen={currentGroupId === id || isOpen}
-            actions={
-              currentGroupId !== id && canView
-                ? // this is inacurate, but public groups are visible to students who cannot see detail until they join
-                  this.renderButtons(id, organizational || isPublic)
-                : undefined
-            }
-          >
-            {this.renderChildGroups(childGroups, actualVisibleGroupsMap)}
-          </TreeViewItem>}
-        {level === 0 &&
-          this.renderChildGroups(childGroups, actualVisibleGroupsMap)}
+        {level !== 0 || onAncestralPath
+          ? <TreeViewItem
+              title={conditionalEmphasize(
+                <GroupsName
+                  id={groupId}
+                  name={name}
+                  localizedTexts={localizedTexts}
+                  noLink
+                />,
+                currentGroupId === groupId
+              )}
+              id={groupId}
+              level={level}
+              admins={primaryAdminsIds}
+              organizational={organizational}
+              isPublic={isPublic}
+              forceOpen={onAncestralPath}
+              isOpen={currentGroupId === groupId || isOpen}
+              actions={
+                currentGroupId !== groupId && canView
+                  ? // this is inacurate, but public groups are visible to students who cannot see detail until they join
+                    this.renderButtons(groupId, organizational || isPublic)
+                  : undefined
+              }
+            >
+              {onAncestralPath
+                ? [
+                    <GroupTree
+                      {...this.props}
+                      key={groupId}
+                      level={level + 1}
+                      ancestralPath={ancestralPath.slice(1)}
+                    />
+                  ]
+                : this.renderChildGroups(
+                    childGroups,
+                    actualVisibleGroupsMap,
+                    level
+                  )}
+            </TreeViewItem>
+          : this.renderChildGroups(childGroups, actualVisibleGroupsMap, 0)}
       </TreeView>
     );
   }
@@ -153,6 +180,7 @@ GroupTree.propTypes = {
   isPublic: PropTypes.bool,
   currentGroupId: PropTypes.string,
   visibleGroupsMap: PropTypes.object,
+  ancestralPath: PropTypes.array,
   links: PropTypes.object,
   intl: PropTypes.shape({ locale: PropTypes.string.isRequired }).isRequired
 };

diff --git a/src/components/Groups/ResultsTable/ResultsTable.js b/src/components/Groups/ResultsTable/ResultsTable.js
@@ -45,14 +45,18 @@ const tableStyles = {
 };
 
 // Create comparators object based on given locale ...
-const prepareTableComparators = defaultMemoize(locale => ({
-  user: ({ user: u1 }, { user: u2 }) =>
+const prepareTableComparators = defaultMemoize(locale => {
+  const nameComparator = (u1, u2) =>
     u1.name.lastName.localeCompare(u2.name.lastName, locale) ||
     u1.name.firstName.localeCompare(u2.name.firstName, locale) ||
-    u1.privateData.email.localeCompare(u2.privateData.email, locale),
-  total: ({ total: t1 }, { total: t2 }) =>
-    (Number(t2.gained) || -1) - (Number(t1.gained) || -1)
-}));
+    u1.privateData.email.localeCompare(u2.privateData.email, locale);
+  return {
+    user: ({ user: u1 }, { user: u2 }) => nameComparator(u1, u2),
+    total: ({ total: t1, user: u1 }, { total: t2, user: u2 }) =>
+      (Number(t1 && t1.gained) || -1) - (Number(t1 && t1.gained) || -1) ||
+      nameComparator(u1, u2)
+  };
+});
 
 class ResultsTable extends Component {
   // Prepare header descriptor object for SortableTable.
@@ -87,7 +91,17 @@ class ResultsTable extends Component {
 
   // Re-format the data, so they can be rendered by the SortableTable ...
   prepareData = defaultMemoize((assignments, users, stats) => {
-    const { isAdmin, renderActions } = this.props;
+    const {
+      isAdmin,
+      isSupervisor,
+      loggedUser,
+      publicStats,
+      renderActions
+    } = this.props;
+
+    if (!isAdmin && !isSupervisor && !publicStats) {
+      users = users.filter(({ id }) => id === loggedUser);
+    }
 
     return users.map(user => {
       const userStats = stats.find(stat => stat.userId === user.id);
@@ -140,8 +154,11 @@ class ResultsTable extends Component {
 ResultsTable.propTypes = {
   assignments: PropTypes.array.isRequired,
   users: PropTypes.array.isRequired,
+  loggedUser: PropTypes.string,
   stats: PropTypes.array.isRequired,
+  publicStats: PropTypes.bool,
   isAdmin: PropTypes.bool,
+  isSupervisor: PropTypes.bool,
   renderActions: PropTypes.func,
   intl: PropTypes.shape({ locale: PropTypes.string.isRequired }).isRequired,
   links: PropTypes.object

diff --git a/src/components/forms/Fields/TabbedArrayField.js b/src/components/forms/Fields/TabbedArrayField.js
@@ -6,6 +6,11 @@ import Button from '../../widgets/FlatButton';
 import Confirm from '../Confirm';
 import { AddIcon, WarningIcon } from '../../icons';
 
+const titleComparator = ({ title: a }, { title: b }) =>
+  typeof a !== 'string'
+    ? typeof b !== 'string' ? 0 : 1
+    : typeof b !== 'string' ? -1 : a.localeCompare(b);
+
 class TabbedArrayField extends Component {
   state = { activeTab: 0 };
   changeTab = n => this.setState({ activeTab: n });
@@ -59,30 +64,39 @@ class TabbedArrayField extends Component {
             activeKey={this.state.activeTab}
             onSelect={this.changeTab}
           >
-            {fields.map((prefix, i) =>
-              <Tab key={i} eventKey={i} title={getTitle(i)}>
-                <ContentComponent {...props} i={i} prefix={prefix} />
-                {remove &&
-                  <p className="text-center">
-                    <Confirm
-                      id={`${id}-remove-${i}`}
-                      question={removeQuestion}
-                      onConfirmed={() => {
-                        fields.remove(i);
-                        this.changeTab(Math.min(i, fields.length - 2));
-                      }}
-                    >
-                      <Button bsStyle="default">
-                        <WarningIcon gapRight />
-                        <FormattedMessage
-                          id="app.tabbedArrayField.remove"
-                          defaultMessage="Remove"
-                        />
-                      </Button>
-                    </Confirm>
-                  </p>}
-              </Tab>
-            )}
+            {fields
+              .map((prefix, i) => ({ prefix, i, title: getTitle(i) }))
+              .sort(titleComparator)
+              .map(({ prefix, i, title }) =>
+                <Tab
+                  key={i}
+                  eventKey={i}
+                  title={title}
+                  mountOnEnter
+                  unmountOnExit
+                >
+                  <ContentComponent {...props} i={i} prefix={prefix} />
+                  {remove &&
+                    <p className="text-center">
+                      <Confirm
+                        id={`${id}-remove-${i}`}
+                        question={removeQuestion}
+                        onConfirmed={() => {
+                          fields.remove(i);
+                          this.changeTab(Math.min(i, fields.length - 2));
+                        }}
+                      >
+                        <Button bsStyle="default">
+                          <WarningIcon gapRight />
+                          <FormattedMessage
+                            id="app.tabbedArrayField.remove"
+                            defaultMessage="Remove"
+                          />
+                        </Button>
+                      </Confirm>
+                    </p>}
+                </Tab>
+              )}
           </Tabs>}
 
         {fields.length === 0 &&

diff --git a/src/components/forms/ForkExerciseForm/ForkExerciseForm.js b/src/components/forms/ForkExerciseForm/ForkExerciseForm.js
@@ -78,18 +78,16 @@ class ForkExerciseForm extends Component {
                     name={'groupId'}
                     component={SelectField}
                     label={''}
-                    options={[{ key: '', name: '_Public_' }].concat(
-                      groups
-                        .map(group => ({
-                          key: group.id,
-                          name: getGroupCanonicalLocalizedName(
-                            group,
-                            groupsAccessor,
-                            locale
-                          )
-                        }))
-                        .sort((a, b) => a.name.localeCompare(b.name, locale))
-                    )}
+                    options={groups
+                      .map(group => ({
+                        key: group.id,
+                        name: getGroupCanonicalLocalizedName(
+                          group,
+                          groupsAccessor,
+                          locale
+                        )
+                      }))
+                      .sort((a, b) => a.name.localeCompare(b.name, locale))}
                   />}
               </ResourceRenderer>
 

diff --git a/src/components/forms/LocalizedTextsFormField/LocalizedTextsFormField.js b/src/components/forms/LocalizedTextsFormField/LocalizedTextsFormField.js
@@ -9,41 +9,44 @@ const LocalizedTextsFormField = ({
   localizedTextsLocales = [],
   isGroup = false,
   ...props
-}) =>
-  <TabbedArrayField
-    {...props}
-    getTitle={i =>
-      localizedTextsLocales && localizedTextsLocales[i]
-        ? localizedTextsLocales[i]
-        : <FormattedMessage
-            id="app.editLocalizedTextForm.newLocale"
-            defaultMessage="New language"
-          />}
-    ContentComponent={
-      isGroup ? LocalizedGroupFormField : LocalizedExerciseFormField
-    }
-    emptyMessage={
-      <FormattedMessage
-        id="app.editLocalizedTextForm.localized.noLanguage"
-        defaultMessage="There is currently no text in any language."
-      />
-    }
-    addMessage={
-      <FormattedMessage
-        id="app.editLocalizedTextForm.addLanguage"
-        defaultMessage="Add language variant"
-      />
-    }
-    removeQuestion={
-      <FormattedMessage
-        id="app.editLocalizedTextForm.localized.reallyRemoveQuestion"
-        defaultMessage="Do you really want to delete this localization?"
-      />
-    }
-    id="localized-texts"
-    add
-    remove
-  />;
+}) => {
+  return (
+    <TabbedArrayField
+      {...props}
+      getTitle={i =>
+        localizedTextsLocales && localizedTextsLocales[i]
+          ? localizedTextsLocales[i]
+          : <FormattedMessage
+              id="app.editLocalizedTextForm.newLocale"
+              defaultMessage="New language"
+            />}
+      ContentComponent={
+        isGroup ? LocalizedGroupFormField : LocalizedExerciseFormField
+      }
+      emptyMessage={
+        <FormattedMessage
+          id="app.editLocalizedTextForm.localized.noLanguage"
+          defaultMessage="There is currently no text in any language."
+        />
+      }
+      addMessage={
+        <FormattedMessage
+          id="app.editLocalizedTextForm.addLanguage"
+          defaultMessage="Add language variant"
+        />
+      }
+      removeQuestion={
+        <FormattedMessage
+          id="app.editLocalizedTextForm.localized.reallyRemoveQuestion"
+          defaultMessage="Do you really want to delete this localization?"
+        />
+      }
+      id="localized-texts"
+      add
+      remove
+    />
+  );
+};
 
 LocalizedTextsFormField.propTypes = {
   localizedTextsLocales: PropTypes.array,

diff --git a/src/components/widgets/TreeView/TreeViewInnerNode.js b/src/components/widgets/TreeView/TreeViewInnerNode.js
@@ -24,8 +24,12 @@ class TreeViewInnerNode extends Component {
         <TreeViewLeaf
           {...props}
           loading={loading}
-          onClick={this.toggleOpen}
-          icon={this.isOpen() ? 'minus-square' : 'plus-square'}
+          onClick={!this.props.forceOpen ? this.toggleOpen : undefined}
+          icon={
+            this.props.forceOpen
+              ? 'square'
+              : this.isOpen() ? 'minus-square' : 'plus-square'
+          }
         />
         <Collapse isOpened={this.isOpen()}>
           {children}