Skip to content

Commit

Permalink
Merge pull request #7076 from Sage/FE-6897
Browse files Browse the repository at this point in the history
feat: remove focusRedesignOptOut feature flag
  • Loading branch information
tomdavies73 authored Dec 19, 2024
2 parents e2c1dd9 + fdcfc6b commit 0f1fbd1
Show file tree
Hide file tree
Showing 83 changed files with 394 additions and 2,398 deletions.
13 changes: 0 additions & 13 deletions .storybook/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,6 @@ const globalTypes = {
],
},
},
focusRedesign: {
name: "Double focus style",
description: "Toggle the focus styling redesign",
defaultValue: "on",
toolbar: {
icon: "eye",
title: "Focus redesign",
items: [
{ value: "on", title: "On" },
{ value: "off", title: "Off" },
],
},
},
...globalThemeProvider,
};

Expand Down
18 changes: 3 additions & 15 deletions .storybook/withThemeProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,8 @@ const themes = [noTheme, sageTheme].reduce((themesObject, theme) => {
return themesObject;
}, {});

const render = (
Story,
themeName,
focusRedesignOptOut,
) => (
<CarbonProvider
theme={themes[themeName]}
focusRedesignOptOut={focusRedesignOptOut}
>
const render = (Story, themeName) => (
<CarbonProvider theme={themes[themeName]}>
<Story themeName={themeName} />
</CarbonProvider>
);
Expand Down Expand Up @@ -58,11 +51,7 @@ const withThemeProvider = makeDecorator({
{Object.keys(themes).map((themeName) => (
<div key={themeName}>
<h3>{themeName}</h3>
{render(
Story,
themeName,
context.globals.focusRedesign === "off",
)}
{render(Story, themeName)}
</div>
))}
</Wrapper>
Expand All @@ -74,7 +63,6 @@ const withThemeProvider = makeDecorator({
isChromaticBuild && chromaticTheme
? chromaticTheme
: context.globals.theme,
context.globals.focusRedesign === "off",
);
},
});
Expand Down
3 changes: 0 additions & 3 deletions playwright/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import "../src/style/fonts.css";
import * as dateLocales from "./support/date-fns-locales";

export type HooksConfig = {
focusRedesignOptOut?: boolean;
validationRedesignOptIn?: boolean;
theme?: string;
localeName?: keyof typeof dateLocales;
Expand Down Expand Up @@ -42,15 +41,13 @@ const mountedTheme = (theme: string) => {
// Setup required providers on mounted component before running test. See https://playwright.dev/docs/test-components#hooks
beforeMount<HooksConfig>(async ({ App, hooksConfig }) => {
const {
focusRedesignOptOut,
theme = "sage",
localeName,
validationRedesignOptIn,
} = hooksConfig || {};
return (
<CarbonProvider
theme={mountedTheme(theme)}
focusRedesignOptOut={focusRedesignOptOut}
validationRedesignOptIn={validationRedesignOptIn}
>
<GlobalStyle />
Expand Down
15 changes: 2 additions & 13 deletions src/__internal__/input-icon-toggle/input-icon-toggle.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ import sizes from "../input/input-sizes.style";
import { ValidationProps } from "../validations";
import addFocusStyling from "../../style/utils/add-focus-styling";

const oldFocusStyling = `
outline: solid 3px var(--colorsSemanticFocus500);
`;

export interface InputIconToggleStyleProps extends ValidationProps {
size?: "small" | "medium" | "large";
disabled?: boolean;
Expand Down Expand Up @@ -51,16 +47,9 @@ const InputIconToggleStyle = styled.span.attrs(
cursor: default;
`}
${({ theme }) =>
`
&:focus {
${
!theme.focusRedesignOptOut
? addFocusStyling()
: /* istanbul ignore next */ oldFocusStyling
}
}
`}
${addFocusStyling()}
}
`;

export default InputIconToggleStyle;
10 changes: 2 additions & 8 deletions src/__internal__/input/input-presentation.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ import { InputContextProps } from "../input-behaviour";
import { CarbonProviderProps } from "../../components/carbon-provider";
import addFocusStyling from "../../style/utils/add-focus-styling";

const oldFocusStyling = `
outline: solid 3px var(--colorsSemanticFocus500);
`;

export const StyledInputPresentationContainer = styled.div<
Pick<CommonInputPresentationProps, "inputWidth" | "maxWidth">
>`
Expand Down Expand Up @@ -118,13 +114,11 @@ const InputPresentationStyle = styled.div<
cursor: not-allowed;
`}
${({ hasFocus, theme }) =>
${({ hasFocus }) =>
hasFocus &&
css`
& {
${!theme.focusRedesignOptOut
? addFocusStyling()
: /* istanbul ignore next */ oldFocusStyling}
${addFocusStyling()}
z-index: 2;
}
`}
Expand Down
9 changes: 1 addition & 8 deletions src/__internal__/validations/validation-icon.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ const validationIconTypes = {
warning: "var(--colorsSemanticCaution500)",
};

const oldFocusStyling = `
outline: solid 2px var(--colorsSemanticFocus500);
`;

type ValidationType = "error" | "warning" | "info";

const ValidationIconStyle = styled.span<
Expand All @@ -40,10 +36,7 @@ const ValidationIconStyle = styled.span<
}
${StyledIcon}:focus {
${({ theme }) =>
!theme.focusRedesignOptOut
? addFocusStyling()
: /* istanbul ignore next */ oldFocusStyling}
${addFocusStyling()}
}
${margin}
Expand Down
114 changes: 20 additions & 94 deletions src/components/accordion/accordion.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
import {
positionOfElement,
getRotationAngle,
checkGoldenOutline,
assertCssValueIsApproximately,
getStyle,
expectEventWasCalledOnce,
Expand All @@ -22,7 +21,6 @@ import {
ACCORDION_REMOVE_CONTENT,
} from "../../../playwright/components/accordion/locators";
import { SIZE, CHARACTERS } from "../../../playwright/support/constants";
import { HooksConfig } from "../../../playwright";

import {
AccordionComponent,
Expand All @@ -42,42 +40,23 @@ import {

const testData = [CHARACTERS.DIACRITICS, CHARACTERS.SPECIALCHARACTERS];

test.describe("when focused", () => {
// TODO: Skipped due to flaky focus behaviour. To review in FE-6428
test.skip("should have the expected styling when the focusRedesignOptOut is false", async ({
mount,
page,
}) => {
await mount(<AccordionComponent />);
const elementLocator = accordionTitleContainer(page);
const element = await elementLocator;
await element.focus();
await expect(elementLocator).toHaveCSS(
"box-shadow",
"rgb(255, 188, 25) 0px 0px 0px 3px, rgba(0, 0, 0, 0.9) 0px 0px 0px 6px",
);
await expect(elementLocator).toHaveCSS(
"outline",
"rgba(0, 0, 0, 0) solid 3px",
);
});

// TODO: Skipped due to flaky focus behaviour. To review in FE-6428
test.skip("should have the expected styling when the focusRedesignOptOut is true", async ({
mount,
page,
}) => {
await mount<HooksConfig>(<AccordionComponent />, {
hooksConfig: { focusRedesignOptOut: true },
});
const elementLocator = accordionTitleContainer(page);
const element = await elementLocator;
await element.focus();
await expect(elementLocator).toHaveCSS(
"outline",
"rgb(255, 188, 25) solid 3px",
);
});
// TODO: Skipped due to flaky focus behaviour. To review in FE-6428
test.skip("should have the expected styling when focused", async ({
mount,
page,
}) => {
await mount(<AccordionComponent />);
const elementLocator = accordionTitleContainer(page);
const element = await elementLocator;
await element.focus();
await expect(elementLocator).toHaveCSS(
"box-shadow",
"rgb(255, 188, 25) 0px 0px 0px 3px, rgba(0, 0, 0, 0.9) 0px 0px 0px 6px",
);
await expect(elementLocator).toHaveCSS(
"outline",
"rgba(0, 0, 0, 0) solid 3px",
);
});

test.describe("should render Accordion component", () => {
Expand Down Expand Up @@ -458,28 +437,7 @@ test.describe("should render Accordion component", () => {
});

test.describe("should render Accordion Grouped component", () => {
test("should move through all grouped accordions using ArrowDown key and check focus when focusRedesignOptOut is true", async ({
mount,
page,
}) => {
await mount<HooksConfig>(<AccordionGroupComponent />, {
hooksConfig: { focusRedesignOptOut: true },
});

await accordionTitleContainer(page).nth(0).focus();
await checkGoldenOutline(accordionTitleContainer(page).nth(0));
await expect(accordionTitleContainer(page).nth(0)).toBeVisible();

await accordionTitleContainer(page).nth(0).press("ArrowDown");
await checkGoldenOutline(accordionTitleContainer(page).nth(1));
await expect(accordionTitleContainer(page).nth(1)).toBeVisible();

await accordionTitleContainer(page).nth(1).press("ArrowDown");
await checkGoldenOutline(accordionTitleContainer(page).nth(2));
await expect(accordionTitleContainer(page).nth(2)).toBeVisible();
});

test("should move through all grouped accordions using ArrowDown key and check focus when focusRedesignOptOut is false", async ({
test("should move through all grouped accordions using ArrowDown key and check focus", async ({
mount,
page,
}) => {
Expand Down Expand Up @@ -507,23 +465,7 @@ test.describe("should render Accordion Grouped component", () => {
await expect(accordionTitleContainer(page).nth(2)).toBeVisible();
});

test("should move to the last grouped accordion using End key and check it is focused when focusRedesignOptOut is true", async ({
mount,
page,
}) => {
await mount<HooksConfig>(<AccordionGroupComponent />, {
hooksConfig: { focusRedesignOptOut: true },
});

await accordionTitleContainer(page).nth(0).focus();

await accordionTitleContainer(page).nth(0).press("End");

await checkGoldenOutline(accordionTitleContainer(page).nth(2));
await expect(accordionTitleContainer(page).nth(2)).toBeVisible();
});

test("should move to the last grouped accordion using End key and check it is focused when focusRedesignOptOut is false", async ({
test("should move to the last grouped accordion using End key and check it is focused", async ({
mount,
page,
}) => {
Expand All @@ -540,23 +482,7 @@ test.describe("should render Accordion Grouped component", () => {
await expect(accordionTitleContainer(page).nth(2)).toBeVisible();
});

test("should move to the first grouped accordion using Home key and check it is focused when focusRedesignOptOut is true", async ({
mount,
page,
}) => {
await mount<HooksConfig>(<AccordionGroupComponent />, {
hooksConfig: { focusRedesignOptOut: true },
});

await accordionTitleContainer(page).nth(2).focus();

await accordionTitleContainer(page).nth(2).press("Home");

await checkGoldenOutline(accordionTitleContainer(page).nth(0));
await expect(accordionTitleContainer(page).nth(2)).toBeVisible();
});

test("should move to the first grouped accordion using Home key and check it is focused when focusRedesignOptOut is false", async ({
test("should move to the first grouped accordion using Home key and check it is focused", async ({
mount,
page,
}) => {
Expand Down
9 changes: 1 addition & 8 deletions src/components/accordion/accordion.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,13 @@ interface StyledAccordionTitleContainerProps {
variant?: "standard" | "subtle";
}

const oldFocusStyling = `
outline: solid 3px var(--colorsSemanticFocus500);
`;

const StyledAccordionTitleContainer = styled.div<StyledAccordionTitleContainerProps>`
${({
buttonHeading,
buttonWidth,
iconAlign,
size,
hasButtonProps,
theme,
isExpanded,
variant,
}) => css`
Expand All @@ -177,9 +172,7 @@ const StyledAccordionTitleContainer = styled.div<StyledAccordionTitleContainerPr
z-index: 1;
&:focus {
${!theme.focusRedesignOptOut
? addFocusStyling()
: /* istanbul ignore next */ oldFocusStyling}
${addFocusStyling()}
}
${variant === "subtle" &&
Expand Down
Loading

0 comments on commit 0f1fbd1

Please sign in to comment.