From 50a91ddb2a004bcd95f8f200e350c87ff53b2178 Mon Sep 17 00:00:00 2001 From: "cursor[bot]" <206951365+cursor[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:52:25 -0700 Subject: [PATCH] [MM-70833] Stop dimming the Classification row in Attribute Management (#38693) --- .../global_attributes_listing.spec.ts | 41 ++++++------- .../global_attributes_table.scss | 14 ----- .../global_attributes_table.test.tsx | 26 ++++++-- .../global_attributes_table.tsx | 60 +++---------------- 4 files changed, 50 insertions(+), 91 deletions(-) diff --git a/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_listing.spec.ts b/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_listing.spec.ts index a35afda5e54..ca0e78c0a50 100644 --- a/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_listing.spec.ts +++ b/e2e-tests/playwright/specs/functional/system_console/global_attributes/global_attributes_listing.spec.ts @@ -10,7 +10,7 @@ * Professional-only licenses hide this admin route (React Router redirects away). */ -import {expect, test, getAdminClient, licenseTier} from '@mattermost/playwright-lib'; +import {expect, test, getAdminClient} from '@mattermost/playwright-lib'; import { CLASSIFICATION_MARKINGS_ADMIN_PATH, @@ -303,8 +303,9 @@ test.describe('System Console - Global Attributes listing', {tag: '@system_conso /** * @objective Ensure a real Classification Markings field (name/object_type/group_id * matching production's saveCreateField) renders the read-only subtitle and an - * open-in-new link to its own admin page instead of the ordinary dot-menu, and that an - * unrelated field — including one that shares the same 'rank' type — is entirely unaffected. + * open-in-new link to its own admin page instead of the ordinary dot-menu, that its name + * still renders at the same contrast as any other row's, and that an unrelated field — + * including one that shares the same 'rank' type — is entirely unaffected. */ test( 'renders the Classification Markings row as a read-only open-in-new link, leaving an unrelated rank field unaffected', @@ -312,17 +313,6 @@ test.describe('System Console - Global Attributes listing', {tag: '@system_conso async ({pw}) => { const {adminUser, adminClient} = await requireGlobalAttributesEnabled(pw); - // Mirrors useClassificationAttributePageReachable (global_attributes_table.tsx): - // Enterprise Advanced tier + the ChannelAttributes flag together make the - // classification attribute page reachable, which adds a dot-menu (Edit there) - // alongside the row's always-present open-in-new link. - const [license, config] = await Promise.all([ - adminClient.getClientLicenseOld(), - adminClient.getConfig(), - ]); - const classificationAttributePageReachable = - licenseTier(license.SkuShortName) >= 30 && config.FeatureFlags.ChannelAttributes === true; - // # The link's destination page is gated by its own independent feature flag // (ClassificationMarkings) — it must be on for the link to render. // Tagged @classification_markings like every other spec that touches this same @@ -391,17 +381,12 @@ test.describe('System Console - Global Attributes listing', {tag: '@system_conso 'Classification Markings', ); - // * The rightmost cell always carries an open-in-new link to the - // Classification Markings admin page + // * The rightmost cell carries only an open-in-new link to the + // Classification Markings admin page — no Edit/More-actions menu const openInNewLink = classificationRow.getByRole('link', {name: 'Open Classification Markings'}); await expect(openInNewLink).toBeVisible(); await expect(openInNewLink).toHaveAttribute('href', CLASSIFICATION_MARKINGS_ADMIN_PATH); - - // * A dot-menu (offering Edit to the classification attribute page) appears - // alongside the open-in-new link only when that page is actually reachable. - await expect(classificationRow.getByRole('button', {name: 'More actions'})).toHaveCount( - classificationAttributePageReachable ? 1 : 0, - ); + await expect(classificationRow.getByRole('button', {name: 'More actions'})).toHaveCount(0); // # Clicking the link actually navigates to the Classification Markings page await openInNewLink.click(); @@ -421,6 +406,18 @@ test.describe('System Console - Global Attributes listing', {tag: '@system_conso await expect(unrelatedRow.getByText('Read-only')).toHaveCount(0); await expect(unrelatedRow.getByRole('link', {name: 'Open Classification Markings'})).toHaveCount(0); await expect(unrelatedRow.getByTestId('global-attribute-source')).toContainText('Managed here'); + + // * Classification's name renders at the same contrast as any other row's: + // a definition that lives on another page must not read as a disabled + // attribute. Compared against the sibling row rather than a literal colour + // so it holds under every theme. + const unrelatedNameColor = await unrelatedRow + .getByTestId('global-attribute-name') + .evaluate((el) => getComputedStyle(el).color); + await expect(classificationRow.getByTestId('global-attribute-name')).toHaveCSS( + 'color', + unrelatedNameColor, + ); } finally { await deleteClassificationMarkingsFieldIfExists(adminClient); await deleteGlobalAttributeFieldIfExists(adminClient, unrelatedRankName); diff --git a/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.scss b/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.scss index fe035e8828a..fbf1e9c1634 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.scss +++ b/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.scss @@ -65,26 +65,12 @@ white-space: nowrap; } -.GlobalAttributesTable__name--classification { - color: rgba(var(--center-channel-color-rgb), 0.56); -} - .GlobalAttributesTable__subtitle { color: rgba(var(--center-channel-color-rgb), 0.64); font-size: 12px; white-space: normal; } -.GlobalAttributesTable__subtitle--classification { - color: rgba(var(--center-channel-color-rgb), 0.56); -} - -.GlobalAttributesTable__actions--classification { - display: inline-flex; - align-items: center; - justify-content: flex-end; -} - .GlobalAttributesTable__link--classification { display: inline-flex; width: 32px; diff --git a/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.test.tsx b/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.test.tsx index 5760a83de6d..0d95c8ef64a 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.test.tsx +++ b/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.test.tsx @@ -1452,7 +1452,7 @@ describe('GlobalAttributesTable', () => { }); describe('Classification Markings row', () => { - it('renders the subtitle, an open-in-new link and a menu carrying Edit when the field matches and the destination is reachable', async () => { + it('renders the subtitle and an open-in-new link with no actions menu when the field matches and the destination is reachable', async () => { getPropertyFields.mockResolvedValueOnce([makeClassificationField()]).mockResolvedValue([]); renderWithContext(, getReachableState()); @@ -1464,9 +1464,9 @@ describe('GlobalAttributesTable', () => { expect(link).toHaveAttribute('href', CLASSIFICATIONS_MARKINGS_ADMIN_URL); expect(link).toHaveAccessibleName('Open Classification Markings'); - // * Unlike every other row, this one's Edit is live: it is the only way to - // configure classification's resources outside the API - expect(screen.getByTestId('global-attribute-actions-field-1')).toBeInTheDocument(); + // * The definition is edited on Classification Markings; this row must not + // offer a duplicate Edit/More-actions menu next to the open-in-new link + expect(screen.queryByTestId('global-attribute-actions-field-1')).not.toBeInTheDocument(); // * The Source column also identifies this row's true source, rather than the // generic "Managed here" every other native field gets @@ -1534,6 +1534,24 @@ describe('GlobalAttributesTable', () => { expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); }); + it('does not dim the name or subtitle, so a read-only definition does not read as a disabled attribute', async () => { + getPropertyFields. + mockResolvedValueOnce([makeClassificationField(), makeField({id: 'field-2', attrs: {display_name: 'Program'}})]). + mockResolvedValue([]); + + renderWithContext(, getReachableState()); + + const [classificationName, ordinaryName] = await screen.findAllByTestId('global-attribute-name'); + expect(classificationName).toHaveTextContent('Classification'); + expect(ordinaryName).toHaveTextContent('Program'); + + // * Dimming the row on top of its "Definition is read-only" subtitle made a + // correctly configured attribute look disabled + expect(classificationName.className).toBe(ordinaryName.className); + expect(screen.getByTestId('global-attribute-classification-subtitle-field-1').className). + toBe('GlobalAttributesTable__subtitle'); + }); + it('leaves an unrelated field (not matching name/object_type/group_id) entirely unaffected even when the destination is reachable', async () => { getPropertyFields.mockResolvedValueOnce([makeField({type: 'rank'})]).mockResolvedValue([]); diff --git a/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.tsx b/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.tsx index ea63a4ae28c..18ef273b85f 100644 --- a/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.tsx +++ b/webapp/channels/src/components/admin_console/global_attributes/global_attributes_table.tsx @@ -2,7 +2,6 @@ // See LICENSE.txt for license information. import {createColumnHelper, getCoreRowModel, useReactTable, type ColumnDef} from '@tanstack/react-table'; -import classNames from 'classnames'; import type {ComponentType} from 'react'; import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import type {MessageDescriptor} from 'react-intl'; @@ -239,7 +238,7 @@ function AttributeCell({field, isClassificationRow}: ClassificationAwareCellProp return ( {getDisplayName(field)} @@ -247,7 +246,7 @@ function AttributeCell({field, isClassificationRow}: ClassificationAwareCellProp {isClassificationRow && ( @@ -258,14 +257,13 @@ function AttributeCell({field, isClassificationRow}: ClassificationAwareCellProp } type ActionsCellProps = ClassificationAwareCellProps & { - canEditClassification: boolean; isMobileView: boolean; pluginInventoryLoaded: boolean; onDeleteError: (message: string | null) => void; onDeleteModalExited: () => void; }; -function ActionsCell({field, isClassificationRow, canEditClassification, isMobileView, pluginInventoryLoaded, onDeleteError, onDeleteModalExited}: ActionsCellProps) { +function ActionsCell({field, isClassificationRow, isMobileView, pluginInventoryLoaded, onDeleteError, onDeleteModalExited}: ActionsCellProps) { const {formatMessage} = useIntl(); const dispatch = useDispatch(); const promptDelete = useGlobalAttributeFieldDelete(); @@ -295,9 +293,12 @@ function ActionsCell({field, isClassificationRow, canEditClassification, isMobil } }, [dispatch, field.id, formatMessage, onDeleteError]); + // Classification's definition lives on Classification Markings; this row only + // offers the open-in-new link there. Row click still opens the attribute page + // when that route is reachable. if (isClassificationRow) { const classificationLinkLabel = formatMessage(actionsLabels.classificationLink); - const externalLink = ( + return ( ); - - if (!canEditClassification) { - return externalLink; - } - - // Both destinations: Edit configures which resources classification applies - // to, the link goes to where its levels are defined. - return ( -
- , - dataTestId: menuId, - 'aria-label': formatMessage(actionsLabels.tooltip), - }} - menuButtonTooltip={{text: formatMessage(actionsLabels.tooltip)}} - menu={{ - id: `${menuId}-menu`, - 'aria-label': formatMessage(actionsLabels.menuLabel), - }} - anchorOrigin={{vertical: 'bottom', horizontal: 'right'}} - transformOrigin={{vertical: 'top', horizontal: 'right'}} - > - } - labels={} - /> - } - labels={} - /> - - {externalLink} -
- ); } return ( @@ -594,8 +554,7 @@ export default function GlobalAttributesTable({searchQuery = ''}: GlobalAttribut return; } - // Same destinations as the row's Edit/View action: classification has - // its own page (or the markings page when that edit route is hidden). + // Classification has its own page (or the markings page when that edit route is hidden). if (isClassificationMarkingsField(field, groupId) && classificationMarkingsReachable) { getHistory().push(classificationAttributePageReachable ? CLASSIFICATION_ATTRIBUTE_ROUTE : CLASSIFICATIONS_MARKINGS_ADMIN_URL); return; @@ -688,7 +647,6 @@ export default function GlobalAttributesTable({searchQuery = ''}: GlobalAttribut ({ data: rows,