From 6ea4f88b5b0dd4f819c03c04e90b15399662472b Mon Sep 17 00:00:00 2001
From: Eugenia <32821331+jenny-s51@users.noreply.github.com>
Date: Mon, 29 Mar 2021 07:35:13 -0400
Subject: [PATCH] roles(associated roles+attributes): address UX feedback
 (#451)

* alphabetize and sort roles

* change delete to remove

* make col widths stationary

* remove duplicate identifier

* fix lint

* update attributes to match new design

* (-) button working

* remove log

* format

* add attribute wip

* fix attributes revert

* enable add button

* disable attributes button when last field empty

* add init field on role creation

* remove log stmts
---
 .../attribute-form/AttributeForm.tsx          | 41 +++++++++++++------
 .../table-toolbar/KeycloakDataTable.tsx       |  6 ++-
 src/realm-roles/AssociatedRolesTab.tsx        | 37 ++++++++++++++---
 src/realm-roles/RealmRoleTabs.tsx             | 17 ++++----
 4 files changed, 73 insertions(+), 28 deletions(-)

diff --git a/src/components/attribute-form/AttributeForm.tsx b/src/components/attribute-form/AttributeForm.tsx
index 8e2cd7aec1..0467aa99c2 100644
--- a/src/components/attribute-form/AttributeForm.tsx
+++ b/src/components/attribute-form/AttributeForm.tsx
@@ -65,7 +65,12 @@ export const AttributesForm = ({
   const { t } = useTranslation("roles");
 
   const columns = ["Key", "Value"];
-  const watchFirstKey = watch("attributes[0].key", "");
+
+  const watchLast = watch(`attributes[${fields.length - 1}].key`, "");
+
+  if (fields.length === 0) {
+    append({ key: "", value: "" });
+  }
 
   return (
     <>
@@ -137,7 +142,7 @@ export const AttributesForm = ({
                 )}
                 {rowIndex === fields.length - 1 && (
                   <Td key="add-button" id="add-button" dataLabel={columns[2]}>
-                    {fields[rowIndex].key === "" && (
+                    {fields.length !== 1 && (
                       <Button
                         id={`minus-button-${rowIndex}`}
                         aria-label={`remove ${attribute.key} with value ${attribute.value} `}
@@ -148,26 +153,36 @@ export const AttributesForm = ({
                         <MinusCircleIcon />
                       </Button>
                     )}
-                    <Button
-                      aria-label={t("roles:addAttributeText")}
-                      id="plus-icon"
-                      variant="link"
-                      className="kc-attributes__plus-icon"
-                      onClick={() => append({ key: "", value: "" })}
-                      icon={<PlusCircleIcon />}
-                      isDisabled={!formState.isValid}
-                    />
                   </Td>
                 )}
               </Tr>
             ))}
+            <Button
+              aria-label={t("roles:addAttributeText")}
+              id="plus-icon"
+              variant="link"
+              className="kc-attributes__plus-icon"
+              onClick={() => append({ key: "", value: "" })}
+              icon={<PlusCircleIcon />}
+              isDisabled={!watchLast}
+            >
+              {t("roles:addAttributeText")}
+            </Button>
           </Tbody>
         </TableComposable>
         <ActionGroup className="kc-attributes__action-group">
-          <Button variant="primary" type="submit" isDisabled={!watchFirstKey}>
+          <Button
+            variant="primary"
+            type="submit"
+            isDisabled={!formState.isDirty}
+          >
             {t("common:save")}
           </Button>
-          <Button onClick={reset} variant="link">
+          <Button
+            onClick={reset}
+            variant="link"
+            isDisabled={!formState.isDirty}
+          >
             {t("common:revert")}
           </Button>
         </ActionGroup>
diff --git a/src/components/table-toolbar/KeycloakDataTable.tsx b/src/components/table-toolbar/KeycloakDataTable.tsx
index 4782ad7cf4..22b6322ecd 100644
--- a/src/components/table-toolbar/KeycloakDataTable.tsx
+++ b/src/components/table-toolbar/KeycloakDataTable.tsx
@@ -56,7 +56,11 @@ function DataTable<T>({
       }
       canSelectAll={canSelectAll}
       cells={columns.map((column) => {
-        return { ...column, title: t(column.displayKey || column.name) };
+        return {
+          ...column,
+          title: t(column.displayKey || column.name),
+          transforms: column.transforms,
+        };
       })}
       rows={rows}
       actions={actions}
diff --git a/src/realm-roles/AssociatedRolesTab.tsx b/src/realm-roles/AssociatedRolesTab.tsx
index e27b7efd30..0db56c4039 100644
--- a/src/realm-roles/AssociatedRolesTab.tsx
+++ b/src/realm-roles/AssociatedRolesTab.tsx
@@ -21,6 +21,8 @@ import { useAdminClient } from "../context/auth/AdminClient";
 import { RoleFormType } from "./RealmRoleTabs";
 import ClientRepresentation from "keycloak-admin/lib/defs/clientRepresentation";
 import { AliasRendererComponent } from "./AliasRendererComponent";
+import _ from "lodash";
+import { cellWidth } from "@patternfly/react-table";
 
 type AssociatedRolesTabProps = {
   additionalRoles: RoleRepresentation[];
@@ -83,12 +85,23 @@ export const AssociatedRolesTab = ({
     return newRoles;
   };
 
-  const loader = async () => {
+  const alphabetize = (rolesList: RoleRepresentation[]) => {
+    return _.sortBy(rolesList, (role) => role.name?.toUpperCase());
+  };
+
+  const loader = async (first?: number, max?: number, search?: string) => {
     if (isInheritedHidden) {
-      return additionalRoles;
+      const filteredRoles = additionalRoles.filter(
+        (role) =>
+          !search ||
+          role.name?.toLowerCase().includes(search) ||
+          role.description?.toLowerCase().includes(search)
+      );
+      const roles = alphabetize(filteredRoles);
+      return roles;
     }
 
-    const allRoles: Promise<RoleRepresentation[]> = additionalRoles.reduce(
+    const fetchedRoles: Promise<RoleRepresentation[]> = additionalRoles.reduce(
       async (acc: Promise<RoleRepresentation[]>, role) => {
         const resolvedRoles = await acc;
         resolvedRoles.push(role);
@@ -99,7 +112,16 @@ export const AssociatedRolesTab = ({
       Promise.resolve([] as RoleRepresentation[])
     );
 
-    return allRoles;
+    return fetchedRoles.then((results: RoleRepresentation[]) => {
+      const filteredRoles = results.filter(
+        (role) =>
+          !search ||
+          role.name?.toLowerCase().includes(search) ||
+          role.description?.toLowerCase().includes(search)
+      );
+      const roles = alphabetize(filteredRoles);
+      return roles;
+    });
   };
 
   useEffect(() => {
@@ -128,7 +150,7 @@ export const AssociatedRolesTab = ({
   const [toggleDeleteDialog, DeleteConfirm] = useConfirmDialog({
     titleKey: "roles:roleRemoveAssociatedRoleConfirm",
     messageKey: t("roles:roleRemoveAssociatedText"),
-    continueButtonLabel: "common:delete",
+    continueButtonLabel: t("common:remove"),
     continueButtonVariant: ButtonVariant.danger,
     onConfirm: async () => {
       try {
@@ -150,7 +172,7 @@ export const AssociatedRolesTab = ({
     messageKey: t("roles:removeAllAssociatedRolesConfirmDialog", {
       name: parentRole?.name || t("createRole"),
     }),
-    continueButtonLabel: "common:delete",
+    continueButtonLabel: "common:remove",
     continueButtonVariant: ButtonVariant.danger,
     onConfirm: async () => {
       try {
@@ -239,17 +261,20 @@ export const AssociatedRolesTab = ({
               displayKey: "roles:roleName",
               cellRenderer: AliasRenderer,
               cellFormatters: [formattedLinkTableCell(), emptyFormatter()],
+              transforms: [cellWidth(40)],
             },
             {
               name: "containerId",
               displayKey: "roles:inheritedFrom",
               cellRenderer: InheritedRoleName,
               cellFormatters: [emptyFormatter()],
+              transforms: [cellWidth(30)],
             },
             {
               name: "description",
               displayKey: "common:description",
               cellFormatters: [emptyFormatter()],
+              transforms: [cellWidth(30)],
             },
           ]}
           emptyState={
diff --git a/src/realm-roles/RealmRoleTabs.tsx b/src/realm-roles/RealmRoleTabs.tsx
index 2ad8053657..415e626732 100644
--- a/src/realm-roles/RealmRoleTabs.tsx
+++ b/src/realm-roles/RealmRoleTabs.tsx
@@ -92,7 +92,7 @@ export const RealmRoleTabs = () => {
     name: "attributes",
   });
 
-  useEffect(() => append({ key: "", value: "" }), [append, role]);
+  //useEffect(() => append({ key: "", value: "" }), [append, role]);
 
   const save = async () => {
     try {
@@ -130,6 +130,7 @@ export const RealmRoleTabs = () => {
         );
 
         setRole(role);
+        form.reset(role);
       } else {
         let createdRole;
         if (!clientId) {
@@ -277,13 +278,6 @@ export const RealmRoleTabs = () => {
               ]
             : id
             ? [
-                <DropdownItem
-                  key="delete-role"
-                  component="button"
-                  onClick={() => toggleDeleteDialog()}
-                >
-                  {t("deleteRole")}
-                </DropdownItem>,
                 <DropdownItem
                   key="toggle-modal"
                   data-testid="add-roles"
@@ -292,6 +286,13 @@ export const RealmRoleTabs = () => {
                 >
                   {t("addAssociatedRolesText")}
                 </DropdownItem>,
+                <DropdownItem
+                  key="delete-role"
+                  component="button"
+                  onClick={() => toggleDeleteDialog()}
+                >
+                  {t("deleteRole")}
+                </DropdownItem>,
               ]
             : undefined
         }