Merge pull request #3325 from cisagov/litterbox/3280-design-review

#3280: Review member permissions in registrar [LITTERBOX]

src/registrar/assets/src/js/getgov/portfolio-member-page.js

@@ -18,11 +18,11 @@ export function initPortfolioNewMemberPageToggle() {
             const unique_id = `${member_type}-${member_id}`;
 
             let cancelInvitationButton = member_type === "invitedmember" ? "Cancel invitation" : "Remove member";
-            wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `for ${member_name}`);
+            wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `More Options for ${member_name}`);
 
             // This easter egg is only for fixtures that dont have names as we are displaying their emails
             // All prod users will have emails linked to their account
-            MembersTable.addMemberModal(num_domains, member_email || "Samwise Gamgee", member_delete_url, unique_id, wrapperDeleteAction);
+            MembersTable.addMemberDeleteModal(num_domains, member_email || member_name || "Samwise Gamgee", member_delete_url, unique_id, wrapperDeleteAction);
 
             uswdsInitializeModals();
 

src/registrar/assets/src/js/getgov/table-base.js

@@ -93,7 +93,6 @@ export function generateKebabHTML(action, unique_id, modal_button_text, screen_r
         <use xlink:href="/public/img/sprite.svg#delete"></use>
       </svg>` : ''}
       ${modal_button_text}
-      <span class="usa-sr-only">${screen_reader_text}</span>
     </a>
   `;
 
@@ -107,6 +106,7 @@ export function generateKebabHTML(action, unique_id, modal_button_text, screen_r
           class="usa-button usa-button--unstyled usa-button--with-icon usa-accordion__button usa-button--more-actions"
           aria-expanded="false"
           aria-controls="more-actions-${unique_id}"
+          aria-label="${screen_reader_text}"
         >
           <svg class="usa-icon top-2px" aria-hidden="true" focusable="false" role="img" width="24">
             <use xlink:href="/public/img/sprite.svg#more_vert"></use>
@@ -284,15 +284,18 @@ export class BaseTable {
         showElement(dataWrapper);
         hideElement(noSearchResultsWrapper);
         hideElement(noDataWrapper);
+        this.tableAnnouncementRegion.innerHTML = '';
       } else {
         hideElement(dataWrapper);
         showElement(noSearchResultsWrapper);
         hideElement(noDataWrapper);
+        this.tableAnnouncementRegion.innerHTML = this.noSearchResultsWrapper.innerHTML;
       }
     } else {
       hideElement(dataWrapper);
       hideElement(noSearchResultsWrapper);
       showElement(noDataWrapper);
+      this.tableAnnouncementRegion.innerHTML = this.noDataWrapper.innerHTML;
     }
   };
 
@@ -448,6 +451,7 @@ export class BaseTable {
     const baseUrlValue = this.getBaseUrl()?.innerHTML ?? null;
     if (!baseUrlValue) return;
 
+    this.tableAnnouncementRegion.innerHTML = '<p>Loading table.</p>';
     let url = `${baseUrlValue}?${searchParams.toString()}`
     fetch(url)
       .then(response => response.json())
@@ -469,7 +473,6 @@ export class BaseTable {
 
         let dataObjects = this.getDataObjects(data);
         let customTableOptions = this.customizeTable(data);
-
         dataObjects.forEach(dataObject => {
           this.addRow(dataObject, tbody, customTableOptions);
         });

src/registrar/assets/src/js/getgov/table-edit-member-domains.js

@@ -103,26 +103,28 @@ export class EditMemberDomainsTable extends BaseTable {
       disabled = true;
     }
 
+    // uses margin-right-neg-5 as a hack to increase the text-wrapping width on this table
     row.innerHTML = `
-        <td data-label="Selection" data-sort-value="0" class="padding-right-105">
+        <th scope="row" role="rowheader" data-label="Selection" data-sort-value="0" class="padding-right-105">
             <div class="usa-checkbox">
                 <input
                     class="usa-checkbox__input"
                     id="${domain.id}"
                     type="checkbox"
                     name="${domain.name}"
                     value="${domain.id}"
+                    aria-label="${domain.name}"
                     ${checked ? 'checked' : ''}
                     ${disabled ? 'disabled' : ''}
                 />
                 <label class="usa-checkbox__label margin-top-0" for="${domain.id}">
                     <span class="sr-only">${domain.id}</span>
                 </label>
             </div>
-        </td>
+        </th>
         <td data-label="Domain name">
             ${domain.name}
-            ${disabled ? '<span class="display-block margin-top-05 text-gray-50">Domains must have one domain manager. To unassign this member, the domain needs another domain manager.</span>' : ''}
+            ${disabled ? '<span class="display-block margin-top-05 text-gray-50 margin-right-neg-5">Domains must have one domain manager. To unassign this member, the domain needs another domain manager.</span>' : ''}
         </td>
     `;
     tbody.appendChild(row);
@@ -235,7 +237,8 @@ export class EditMemberDomainsTable extends BaseTable {
     // Create unassigned domains list
     const unassignedDomainsList = document.createElement('ul');
     unassignedDomainsList.classList.add('usa-list', 'usa-list--unstyled');
-    this.removedDomains.forEach(removedDomain => {
+    let removedDomainsCopy = [...this.removedDomains].sort((a, b) => a.name.localeCompare(b.name));
+    removedDomainsCopy.forEach(removedDomain => {
         const removedDomainListItem = document.createElement('li');
         removedDomainListItem.textContent = removedDomain.name; // Use textContent for security
         unassignedDomainsList.appendChild(removedDomainListItem);
@@ -244,7 +247,8 @@ export class EditMemberDomainsTable extends BaseTable {
     // Create assigned domains list
     const assignedDomainsList = document.createElement('ul');
     assignedDomainsList.classList.add('usa-list', 'usa-list--unstyled');
-    this.addedDomains.forEach(addedDomain => {
+    let addedDomainsCopy = [...this.addedDomains].sort((a, b) => a.name.localeCompare(b.name));
+    addedDomainsCopy.forEach(addedDomain => {
         const addedDomainListItem = document.createElement('li');
         addedDomainListItem.textContent = addedDomain.name; // Use textContent for security
         assignedDomainsList.appendChild(addedDomainListItem);
@@ -259,7 +263,7 @@ export class EditMemberDomainsTable extends BaseTable {
     // Append unassigned domains section
     if (this.removedDomains.length) {
       const unassignedHeader = document.createElement('h3');
-      unassignedHeader.classList.add('margin-bottom-1');
+      unassignedHeader.classList.add('margin-bottom-05', 'h4');
       unassignedHeader.textContent = 'Unassigned domains';
       domainAssignmentSummary.appendChild(unassignedHeader);
       domainAssignmentSummary.appendChild(unassignedDomainsList);
@@ -268,15 +272,17 @@ export class EditMemberDomainsTable extends BaseTable {
     // Append assigned domains section
     if (this.addedDomains.length) {
       const assignedHeader = document.createElement('h3');
-      assignedHeader.classList.add('margin-bottom-1');
+      // Make this h3 look like a h4
+      assignedHeader.classList.add('margin-bottom-05', 'h4');
       assignedHeader.textContent = 'Assigned domains';
       domainAssignmentSummary.appendChild(assignedHeader);
       domainAssignmentSummary.appendChild(assignedDomainsList);
     }
 
     // Append total assigned domains section
     const totalHeader = document.createElement('h3');
-    totalHeader.classList.add('margin-bottom-1');
+    // Make this h3 look like a h4
+    totalHeader.classList.add('margin-bottom-05', 'h4');
     totalHeader.textContent = 'Total assigned domains';
     domainAssignmentSummary.appendChild(totalHeader);
     const totalCount = document.createElement('p');
@@ -289,6 +295,7 @@ export class EditMemberDomainsTable extends BaseTable {
     this.updateReadonlyDisplay();
     hideElement(this.editModeContainer);
     showElement(this.readonlyModeContainer);
+    window.scrollTo(0, 0);
   }
 
   showEditMode() {

src/registrar/assets/src/js/getgov/table-member-domains.js

@@ -19,9 +19,9 @@ export class MemberDomainsTable extends BaseTable {
     const domain = dataObject;
     const row = document.createElement('tr');
     row.innerHTML = `
-      <td scope="row" data-label="Domain name">
+      <th scope="row" role="rowheader" data-label="Domain name">
         ${domain.name}
-      </td>
+      </th>
     `;
     tbody.appendChild(row);
   }

src/registrar/assets/src/js/getgov/table-members.js

@@ -78,13 +78,12 @@ export class MembersTable extends BaseTable {
     const num_domains = member.domain_urls.length;
     const last_active = this.handleLastActive(member.last_active);
     let cancelInvitationButton = member.type === "invitedmember" ? "Cancel invitation" : "Remove member";
-    const kebabHTML = customTableOptions.hasAdditionalActions ? generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `for ${member.name}`): ''; 
+    const kebabHTML = customTableOptions.hasAdditionalActions ? generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `Expand for more options for ${member.name}`): ''; 
 
     const row = document.createElement('tr');
-
     let admin_tagHTML = ``;
     if (member.is_admin)
-      admin_tagHTML = `<span class="usa-tag margin-left-1 bg-primary">Admin</span>`
+      admin_tagHTML = `<span class="usa-tag margin-left-1 bg-primary-dark text-semibold">Admin</span>`
 
     // generate html blocks for domains and permissions for the member
     let domainsHTML = this.generateDomainsHTML(num_domains, member.domain_names, member.domain_urls, member.action_url);
@@ -99,7 +98,8 @@ export class MembersTable extends BaseTable {
           type="button" 
           class="usa-button--show-more-button usa-button usa-button--unstyled display-block margin-top-1" 
           data-for=${unique_id}
-          aria-label="Expand for additional information"
+          aria-label="Expand for additional information for ${member.member_display}"
+          aria-label-placeholder="${member.member_display}"
         >
           <span>Expand</span>
           <svg class="usa-icon usa-icon--large" aria-hidden="true" focusable="false" role="img" width="24">
@@ -137,7 +137,7 @@ export class MembersTable extends BaseTable {
     }
     // This easter egg is only for fixtures that dont have names as we are displaying their emails
     // All prod users will have emails linked to their account
-    if (customTableOptions.hasAdditionalActions) MembersTable.addMemberDeleteModal(num_domains, member.email || "Samwise Gamgee", member_delete_url, unique_id, row);
+    if (customTableOptions.hasAdditionalActions) MembersTable.addMemberDeleteModal(num_domains, member.email || member.name || "Samwise Gamgee", member_delete_url, unique_id, row);
   }
 
   /**
@@ -166,13 +166,27 @@ export class MembersTable extends BaseTable {
         spanElement.textContent = 'Close';
         useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_less');
         buttonParentRow.classList.add('hide-td-borders');
-        toggleButton.setAttribute('aria-label', 'Close additional information');
+
+        let ariaLabelText = "Close additional information";
+        let ariaLabelPlaceholder = toggleButton.getAttribute("aria-label-placeholder");
+        if (ariaLabelPlaceholder) {
+          ariaLabelText = `Close additional information for ${ariaLabelPlaceholder}`;
+        }
+        toggleButton.setAttribute('aria-label', ariaLabelText);
+
+        // Set tabindex for focusable elements in expanded content
       } else {    
         hideElement(contentDiv);
         spanElement.textContent = 'Expand';
         useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_more');
         buttonParentRow.classList.remove('hide-td-borders');
-        toggleButton.setAttribute('aria-label', 'Expand for additional information');
+
+        let ariaLabelText = "Expand for additional information";
+        let ariaLabelPlaceholder = toggleButton.getAttribute("aria-label-placeholder");
+        if (ariaLabelPlaceholder) {
+          ariaLabelText = `Expand for additional information for ${ariaLabelPlaceholder}`;
+        }
+        toggleButton.setAttribute('aria-label', ariaLabelText);
       }
     }
 
@@ -245,21 +259,19 @@ export class MembersTable extends BaseTable {
     // Only generate HTML if the member has one or more assigned domains
     if (num_domains > 0) {
       domainsHTML += "<div class='desktop:grid-col-5 margin-bottom-2 desktop:margin-bottom-0'>";
-      domainsHTML += "<h4 class='margin-y-0'>Domains assigned</h4>";
-      domainsHTML += `<p class='margin-y-0'>This member is assigned to ${num_domains} domains:</p>`;
+      domainsHTML += "<h4 class='font-body-xs margin-y-0'>Domains assigned</h4>";
+      domainsHTML += `<p class='font-body-xs text-base-dark margin-y-0'>This member is assigned to ${num_domains} domain${num_domains > 1 ? 's' : ''}:</p>`;
       domainsHTML += "<ul class='usa-list usa-list--unstyled margin-y-0'>";
 
       // Display up to 6 domains with their URLs
       for (let i = 0; i < num_domains && i < 6; i++) {
-        domainsHTML += `<li><a href="${domain_urls[i]}">${domain_names[i]}</a></li>`;
+        domainsHTML += `<li><a class="font-body-xs" href="${domain_urls[i]}">${domain_names[i]}</a></li>`;
       }
 
       domainsHTML += "</ul>";
 
       // If there are more than 6 domains, display a "View assigned domains" link
-      if (num_domains >= 6) {
-        domainsHTML += `<p><a href="${action_url}/domains">View assigned domains</a></p>`;
-      }
+      domainsHTML += `<p class="font-body-xs"><a href="${action_url}/domains">View assigned domains</a></p>`;
 
       domainsHTML += "</div>";
     }
@@ -378,34 +390,37 @@ export class MembersTable extends BaseTable {
   generatePermissionsHTML(member_permissions, UserPortfolioPermissionChoices) {
     let permissionsHTML = '';
 
+    // Define shared classes across elements for easier refactoring
+    let sharedParagraphClasses = "font-body-xs text-base-dark margin-top-1 p--blockquote";
+
     // Check domain-related permissions
     if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)) {
-      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
+      permissionsHTML += `<p class='${sharedParagraphClasses}'><strong class='text-base-dark'>Domains:</strong> Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>`;
     } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) {
-      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domains:</strong> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>";
+      permissionsHTML += `<p class='${sharedParagraphClasses}'><strong class='text-base-dark'>Domains:</strong> Can manage domains they are assigned to and edit information about the domain (including DNS settings).</p>`;
     }
 
     // Check request-related permissions
     if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_REQUESTS)) {
-      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests:</strong> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>";
+      permissionsHTML += `<p class='${sharedParagraphClasses}'><strong class='text-base-dark'>Domain requests:</strong> Can view all organization domain requests. Can create domain requests and modify their own requests.</p>`;
     } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)) {
-      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Domain requests (view-only):</strong> Can view all organization domain requests. Can't create or modify any domain requests.</p>";
+      permissionsHTML += `<p class='${sharedParagraphClasses}'><strong class='text-base-dark'>Domain requests (view-only):</strong> Can view all organization domain requests. Can't create or modify any domain requests.</p>`;
     }
 
     // Check member-related permissions
     if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) {
-      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members:</strong> Can manage members including inviting new members, removing current members, and assigning domains to members.</p>";
+      permissionsHTML += `<p class='${sharedParagraphClasses}'><strong class='text-base-dark'>Members:</strong> Can manage members including inviting new members, removing current members, and assigning domains to members.</p>`;
     } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) {
-      permissionsHTML += "<p class='margin-top-1 p--blockquote'><strong class='text-base-dark'>Members (view-only):</strong> Can view all organizational members. Can't manage any members.</p>";
+      permissionsHTML += `<p class='${sharedParagraphClasses}'><strong class='text-base-dark'>Members (view-only):</strong> Can view all organizational members. Can't manage any members.</p>`;
     }
 
     // If no specific permissions are assigned, display a message indicating no additional permissions
     if (!permissionsHTML) {
-      permissionsHTML += "<p class='margin-top-1 p--blockquote'><b>No additional permissions:</b> There are no additional permissions for this member.</p>";
+      permissionsHTML += `<p class='${sharedParagraphClasses}'><b>No additional permissions:</b> There are no additional permissions for this member.</p>`;
     }
 
     // Add a permissions header and wrap the entire output in a container
-    permissionsHTML = "<div class='desktop:grid-col-7'><h4 class='margin-y-0'>Additional permissions for this member</h4>" + permissionsHTML + "</div>";
+    permissionsHTML = `<div class='desktop:grid-col-7'><h4 class='font-body-xs margin-y-0'>Additional permissions for this member</h4>${permissionsHTML}</div>`;
 
     return permissionsHTML;
   }
@@ -423,7 +438,7 @@ export class MembersTable extends BaseTable {
     let modalDescription = ``;
 
     if (num_domains >= 0){
-      modalHeading = `Are you sure you want to delete ${member_email}?`;
+      modalHeading = `Are you sure you want to remove ${member_email} from the organization?`;
       modalDescription = `They will no longer be able to access this organization. 
       This action cannot be undone.`;
       if (num_domains >= 1)

src/registrar/assets/src/sass/_theme/_buttons.scss

@@ -246,6 +246,10 @@ a.text-secondary:hover {
   color: $theme-color-error;
 }
 
+.usa-button.usa-button--secondary {
+  background-color: $theme-color-error;
+}
+
 .usa-button--show-more-button {
   font-size: size('ui', 'xs');
   text-decoration: none;

src/registrar/assets/src/sass/_theme/_modals.scss

@@ -0,0 +1,5 @@
+@use "uswds-core" as *;
+
+.usa-modal__main {
+    padding: 0 2rem 2rem;
+}

src/registrar/assets/src/sass/_theme/_tables.scss

@@ -41,6 +41,13 @@ th {
   }
 }
 
+// The member table has an extra "expand" row, which looks like a single row.
+// But the DOM disagrees - so we basically need to hide the border on both rows.
+#members__table-wrapper .dotgov-table tr:nth-last-child(2) td,
+#members__table-wrapper .dotgov-table tr:nth-last-child(2) th {
+  border-bottom: none;
+}
+
 .dotgov-table {
   width: 100%;
 
@@ -56,10 +63,9 @@ th {
       border: none;
     }
 
-    tr:not(.hide-td-borders) {
-      td, th {
-        border-bottom: 1px solid color('base-lighter');
-      }
+    tr:not(.hide-td-borders):not(:last-child) td,
+    tr:not(.hide-td-borders):not(:last-child) th {
+      border-bottom: 1px solid color('base-lighter');
     }
 
     thead th {

src/registrar/assets/src/sass/_theme/_tags.scss

@@ -0,0 +1,3 @@
+.usa-tag {
+    text-transform: none;
+}

src/registrar/assets/src/sass/_theme/_uswds-theme.scss

@@ -68,6 +68,7 @@ in the form $setting: value,
   /*---------------------------
   ## Font weights
   ----------------------------*/ 
+  $theme-font-weight-medium: 400,
   $theme-font-weight-semibold: 600,
 
   /*---------------------------

src/registrar/assets/src/sass/_theme/styles.scss

@@ -26,6 +26,8 @@
 @forward "header";
 @forward "register-form";
 @forward "containers";
+@forward "modals";
+@forward "tags";
 
 /*--------------------------------------------------
 --- Admin ---------------------------------*/