From fc0383620bd21b45980a737fabcfc1ccda24bb22 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 28 Mar 2025 15:59:25 +0100 Subject: [PATCH] fix: focus styles for env headers (#9635) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds focus styles to the env accordion header only when the focus is on the header itself (not on the env toggle inside the header). The focus style is consistent with what we do for other accordions (dashboard, milestones). Middle one is focused: ![image](https://github.com/user-attachments/assets/df87bd99-8fe2-4093-afd8-4cbce9f2c943) Focus is on the toggle inside the top one (yeh, we should have better focus styles for toggles; but that's not for now): ![image](https://github.com/user-attachments/assets/2a046d4c-8585-4021-a58e-32ef81b1f701) Open and focused: ![image](https://github.com/user-attachments/assets/fdbb5bda-4be5-4354-b213-5e2c7a59eb59) Getting the consistent background for the header when it's open is a little tricky because the accordion container and summary are split into different files. ~~This first iteration used a class name for the specific header (because envs can have multiple accordion headers inside them, e.g. release plans) and setting a CSS variable in the summary, so that the background matches.~~ I found out that I only need to set it in the parent anyway 😄 Without it, you get this (notice that there is a little white outside the lower corners): ![image](https://github.com/user-attachments/assets/4d71d73c-7f45-46b5-811d-c6e36f9be5ce) --- .../ConstraintAccordionView.tsx | 1 + .../StrategyItemContainer/StrategyItemContainer.tsx | 1 + .../EnvironmentHeader/EnvironmentHeader.tsx | 12 ++++++++++++ .../FeatureOverviewEnvironment.tsx | 13 ++++++------- .../FeatureOverview/ReleasePlan/ReleasePlan.tsx | 6 ++---- 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView.tsx index 6e6b734a91..2712952f06 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView.tsx @@ -95,6 +95,7 @@ export const ConstraintAccordionView = ({ cursor: expandable ? 'pointer' : 'default!important', }, }} + tabIndex={expandable ? 0 : -1} > = ({ {onDragStart ? ( ({ @@ -108,6 +115,9 @@ const MetadataChip = ({ return {text}; }; +export const environmentAccordionSummaryClassName = + 'environment-accordion-summary'; + export const EnvironmentHeader: FC< PropsWithChildren > = ({ @@ -129,6 +139,8 @@ export const EnvironmentHeader: FC< id={id} aria-controls={`environment-accordion-${id}-content`} expandable={expandable} + tabIndex={expandable ? 0 : -1} + className={environmentAccordionSummaryClassName} > diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx index dec4ec9e91..dea0b04ed8 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/FeatureOverviewEnvironment.tsx @@ -8,7 +8,10 @@ import { FEATURE_ENVIRONMENT_ACCORDION } from 'utils/testIds'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { UpgradeChangeRequests } from './UpgradeChangeRequests/UpgradeChangeRequests'; import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; -import { EnvironmentHeader } from './EnvironmentHeader/EnvironmentHeader'; +import { + environmentAccordionSummaryClassName, + EnvironmentHeader, +} from './EnvironmentHeader/EnvironmentHeader'; import FeatureOverviewEnvironmentMetrics from './EnvironmentHeader/FeatureOverviewEnvironmentMetrics/FeatureOverviewEnvironmentMetrics'; import { FeatureOverviewEnvironmentToggle } from './EnvironmentHeader/FeatureOverviewEnvironmentToggle/FeatureOverviewEnvironmentToggle'; import { useState } from 'react'; @@ -26,11 +29,8 @@ const StyledFeatureOverviewEnvironment = styled('div')(({ theme }) => ({ const StyledAccordion = styled(Accordion)(({ theme }) => ({ boxShadow: 'none', background: 'none', - '&&& .MuiAccordionSummary-root': { - borderRadius: theme.shape.borderRadiusLarge, - pointerEvents: 'auto', - opacity: 1, - backgroundColor: theme.palette.background.paper, + [`&:has(.${environmentAccordionSummaryClassName}:focus-visible)`]: { + background: theme.palette.table.headerHover, }, })); @@ -85,7 +85,6 @@ export const FeatureOverviewEnvironment = ({ TransitionProps={{ mountOnEnter: true, unmountOnExit: true }} data-testid={`${FEATURE_ENVIRONMENT_ACCORDION}_${environment.name}`} expanded={isOpen && hasActivations} - disabled={!hasActivations} onChange={() => { const state = isOpen ? !isOpen : hasActivations; onToggleEnvOpen(state); diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlan.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlan.tsx index f4241dd155..952fc547b8 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlan.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlan.tsx @@ -24,9 +24,7 @@ import { StartMilestoneChangeRequestDialog } from './ChangeRequest/StartMileston import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; import { Truncator } from 'component/common/Truncator/Truncator'; -const StyledContainer = styled('div', { - shouldForwardProp: (prop) => prop !== 'readonly', -})<{ readonly?: boolean }>(({ theme, readonly }) => ({ +const StyledContainer = styled('div')(({ theme }) => ({ padding: theme.spacing(2), paddingTop: theme.spacing(0), background: 'inherit', @@ -235,7 +233,7 @@ export const ReleasePlan = ({ ); return ( - +