Skip to content

Commit

Permalink
fix(select): remove list action button from scrollable area in select…
Browse files Browse the repository at this point in the history
… list

Adds a new scrollable container to the select list so that the list action button is no longer
within the scrollable area.

fix #6541
  • Loading branch information
edleeks87 committed Mar 27, 2024
1 parent 886612a commit d95770e
Show file tree
Hide file tree
Showing 19 changed files with 181 additions and 97 deletions.
4 changes: 4 additions & 0 deletions playwright/components/select/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SELECT_ELEMENT_INPUT,
FILTERABLE_ADD_BUTTON,
SELECT_RESET_BUTTON,
SELECT_LIST_SCROLLABLE_WRAPPER,
} from "./locators";
import { PILL_PREVIEW } from "../pill/locators";
import { ALERT_DIALOG } from "../dialog/locators";
Expand Down Expand Up @@ -89,6 +90,9 @@ export const selectListOptionGroup = (page: Page) =>
export const selectListWrapper = (page: Page) =>
page.locator(SELECT_LIST_WRAPPER);

export const selectListScrollableWrapper = (page: Page) =>
page.locator(SELECT_LIST_SCROLLABLE_WRAPPER);

export const selectElementInput = (page: Page) =>
page.locator(SELECT_ELEMENT_INPUT);

Expand Down
2 changes: 2 additions & 0 deletions playwright/components/select/locators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ export const SELECT_LIST_WRAPPER = '[data-element="select-list-wrapper"]';
export const SELECT_ELEMENT_INPUT = '[data-element="select-input"]';
export const FILTERABLE_ADD_BUTTON = '[data-component="button"]';
export const SELECT_RESET_BUTTON = '[data-element="reset-button"]';
export const SELECT_LIST_SCROLLABLE_WRAPPER =
'[data-component="select-list-scrollable-container"]';
28 changes: 28 additions & 0 deletions src/__spec_helper__/mock-dom-rect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
const getDOMRect = (width: number, height: number): DOMRect => ({
width,
height,
top: 0,
left: 0,
bottom: 0,
right: 0,
x: 0,
y: 0,
toJSON: () => {},
});

const mockDOMRect = (
width: number,
height: number,
elementIdentifier: string
) => {
Element.prototype.getBoundingClientRect = jest.fn(function (
this: HTMLElement
) {
if (this.getAttribute("data-component") === elementIdentifier) {
return getDOMRect(width, height);
}
return getDOMRect(0, 0);
});
};

export default mockDOMRect;
2 changes: 1 addition & 1 deletion src/__spec_helper__/select-test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { act } from "react-dom/test-utils";
import type { ReactWrapper } from "enzyme";
import { mockResizeObserver } from "jsdom-testing-mocks";

import StyledSelectListContainer from "../components/select/select-list/select-list-container.style";
import { StyledSelectListContainer } from "../components/select/select-list/select-list.style";

const resizeObserver = mockResizeObserver();

Expand Down
3 changes: 3 additions & 0 deletions src/components/pager/pager.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import StyledOption from "../select/option/option.style";
import Form from "../form";
import StyledFormField from "../../__internal__/form-field/form-field.style";
import I18nProvider from "../i18n-provider";
import mockDOMRect from "../../__spec_helper__/mock-dom-rect";

jest.mock("../../__internal__/utils/helpers/guid");
(guid as jest.MockedFunction<typeof guid>).mockImplementation(
Expand Down Expand Up @@ -596,6 +597,8 @@ describe("Pager", () => {
};

beforeEach(() => {
mockDOMRect(200, 200, "select-list-scrollable-container");

onPagination = jest.fn();

wrapper = render({
Expand Down
23 changes: 12 additions & 11 deletions src/components/select/filterable-select/filterable-select.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
selectInput,
selectList,
selectListPosition,
selectListScrollableWrapper,
selectListWrapper,
selectOption,
selectOptionByText,
Expand Down Expand Up @@ -605,7 +606,7 @@ test.describe("FilterableSelect component", () => {
);
await expect(selectOptionByText(page, option)).toHaveCount(0);
await page.waitForTimeout(2000);
await selectListWrapperElement.evaluate((wrapper) => {
await selectListScrollableWrapper(page).evaluate((wrapper) => {
wrapper.scrollBy(0, 500);
});
await page.waitForTimeout(250);
Expand All @@ -631,15 +632,15 @@ test.describe("FilterableSelect component", () => {

// reopen the list and scroll to initiate the lazy loading. It's important to not use the keyboard here as that
// won't trigger the bug.
const wrapperElement = selectListWrapper(page);
const scrollableWrapper = selectListScrollableWrapper(page);
await dropdownButton(page).click();
await wrapperElement.evaluate((wrapper) => wrapper.scrollBy(0, 500));
const scrollPositionBeforeLoad = await wrapperElement.evaluate(
await scrollableWrapper.evaluate((wrapper) => wrapper.scrollBy(0, 500));
const scrollPositionBeforeLoad = await scrollableWrapper.evaluate(
(element) => element.scrollTop
);

await selectOptionByText(page, "Lazy Loaded A1").waitFor();
const scrollPositionAfterLoad = await wrapperElement.evaluate(
const scrollPositionAfterLoad = await scrollableWrapper.evaluate(
(element) => element.scrollTop
);
await expect(scrollPositionAfterLoad).toBe(scrollPositionBeforeLoad);
Expand Down Expand Up @@ -744,7 +745,7 @@ test.describe("FilterableSelect component", () => {
await mount(<FilterableSelectComponent listMaxHeight={maxHeight} />);

await dropdownButton(page).click();
const wrapperElement = selectListWrapper(page);
const wrapperElement = selectListScrollableWrapper(page);
await expect(wrapperElement).toHaveCSS("max-height", `${maxHeight}px`);
await expect(wrapperElement).toBeVisible();
});
Expand Down Expand Up @@ -1319,7 +1320,7 @@ test.describe("Check events for FilterableSelect component", () => {
await mount(<FilterableSelectComponent onListScrollBottom={callback} />);

await dropdownButton(page).click();
await selectListWrapper(page).evaluate((wrapper) =>
await selectListScrollableWrapper(page).evaluate((wrapper) =>
wrapper.scrollBy(0, 500)
);
await page.waitForTimeout(250);
Expand Down Expand Up @@ -1353,7 +1354,7 @@ test.describe("Check events for FilterableSelect component", () => {
await mount(<FilterableSelectComponent onListScrollBottom={callback} />);

await dropdownButton(page).click();
await selectListWrapper(page).evaluate((wrapper) =>
await selectListScrollableWrapper(page).evaluate((wrapper) =>
wrapper.scrollBy(0, 500)
);
await selectOption(page, positionOfElement("first")).click();
Expand Down Expand Up @@ -1384,7 +1385,7 @@ test.describe("Check virtual scrolling", () => {
await mount(<FilterableSelectWithManyOptionsAndVirtualScrolling />);

await dropdownButton(page).click();
await selectListWrapper(page).evaluate((wrapper) =>
await selectListScrollableWrapper(page).evaluate((wrapper) =>
wrapper.scrollTo(0, 750)
);
await page.waitForTimeout(250);
Expand Down Expand Up @@ -1662,7 +1663,7 @@ test.describe("Test for scroll bug regression", () => {
await mount(<FilterableSelectComponent />);
const dropdownButtonElement = dropdownButton(page);
await dropdownButtonElement.click();
await selectListWrapper(page).evaluate((wrapper) =>
await selectListScrollableWrapper(page).evaluate((wrapper) =>
wrapper.scrollBy(0, 500)
);
await commonDataElementInputPreview(page).press("Escape");
Expand Down Expand Up @@ -1901,7 +1902,7 @@ test.describe("Accessibility tests for FilterableSelect component", () => {
await checkAccessibility(page);
// wait for content to finish loading before scrolling
await expect(selectOptionByText(page, "Amber")).toBeVisible();
await selectListWrapper(page).evaluate((wrapper) =>
await selectListScrollableWrapper(page).evaluate((wrapper) =>
wrapper.scrollBy(0, 500)
);
await checkAccessibility(page, undefined, "scrollable-region-focusable");
Expand Down
12 changes: 10 additions & 2 deletions src/components/select/filterable-select/filterable-select.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,18 @@ import {
import { FilterableSelect, Option, FilterableSelectProps } from "..";
import Textbox from "../../textbox";
import SelectList from "../select-list/select-list.component";
import StyledSelectListContainer from "../select-list/select-list-container.style";
import {
StyledSelectListContainer,
StyledScrollableContainer,
} from "../select-list/select-list.style";
import Button from "../../button";
import Label from "../../../__internal__/label";
import InputIconToggle from "../../../__internal__/input-icon-toggle";
import guid from "../../../__internal__/utils/helpers/guid";
import { InputPresentation } from "../../../__internal__/input";
import Logger from "../../../__internal__/utils/logger";
import StyledInput from "../../../__internal__/input/input.style";
import mockDOMRect from "../../../__spec_helper__/mock-dom-rect";

const mockedGuid = "mocked-guid";
jest.mock("../../../__internal__/utils/logger");
Expand All @@ -47,6 +51,10 @@ function renderSelect(props = {}, renderer = mount, opts = {}) {
describe("FilterableSelect", () => {
let loggerSpy: jest.SpyInstance<void, [message: string]> | jest.Mock;

beforeEach(() => {
mockDOMRect(200, 200, "select-list-scrollable-container");
});

describe("Deprecation warning for uncontrolled", () => {
beforeEach(() => {
loggerSpy = jest.spyOn(Logger, "deprecate");
Expand Down Expand Up @@ -213,7 +221,7 @@ describe("FilterableSelect", () => {
simulateDropdownEvent(wrapper, "click");
assertStyleMatch(
{ maxHeight: "120px" },
wrapper.find(StyledSelectListContainer)
wrapper.find(StyledScrollableContainer)
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ exports[`Option renders properly 1`] = `
position: sticky;
bottom: 0;
background-color: inherit;
border-bottom-left-radius: var(--borderRadius050);
border-bottom-right-radius: var(--borderRadius050);
}
.c0 .c1 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const StyledListActionButtonWrapper = styled.div`
position: sticky;
bottom: 0;
background-color: inherit;
border-bottom-left-radius: var(--borderRadius050);
border-bottom-right-radius: var(--borderRadius050);
${StyledButton} {
border: none;
Expand Down
5 changes: 3 additions & 2 deletions src/components/select/multi-select/multi-select.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
selectInput,
selectList,
selectListPosition,
selectListScrollableWrapper,
selectListWrapper,
selectOption,
selectOptionByText,
Expand Down Expand Up @@ -1302,7 +1303,7 @@ test.describe("Check virtual scrolling", () => {
await mount(<MultiSelectWithManyOptionsAndVirtualScrolling />);

await dropdownButton(page).click();
await selectListWrapper(page).evaluate((wrapper) =>
await selectListScrollableWrapper(page).evaluate((wrapper) =>
wrapper.scrollTo(0, 750)
);
await page.waitForTimeout(250);
Expand Down Expand Up @@ -1584,7 +1585,7 @@ test.describe("Test for scroll bug regression", () => {
await mount(<MultiSelectComponent />);
const dropdownButtonElement = dropdownButton(page);
await dropdownButtonElement.click();
await selectListWrapper(page).evaluate((wrapper) =>
await selectListScrollableWrapper(page).evaluate((wrapper) =>
wrapper.scrollBy(0, 500)
);
await commonDataElementInputPreview(page).press("Escape");
Expand Down
14 changes: 11 additions & 3 deletions src/components/select/multi-select/multi-select.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,19 @@ import {
import { MultiSelect, Option, MultiSelectProps } from "..";
import Textbox from "../../textbox";
import SelectList from "../select-list/select-list.component";
import { StyledSelectList } from "../select-list/select-list.style";
import StyledSelectListContainer from "../select-list/select-list-container.style";
import {
StyledSelectList,
StyledSelectListContainer,
StyledScrollableContainer,
} from "../select-list/select-list.style";
import Pill from "../../pill";
import Label from "../../../__internal__/label";
import InputPresentationStyle from "../../../__internal__/input/input-presentation.style";
import { InputPresentation } from "../../../__internal__/input";
import Logger from "../../../__internal__/utils/logger";
import guid from "../../../__internal__/utils/helpers/guid";
import StyledInput from "../../../__internal__/input/input.style";
import mockDOMRect from "../../../__spec_helper__/mock-dom-rect";

const mockedGuid = "mocked-guid";
jest.mock("../../../__internal__/utils/helpers/guid");
Expand All @@ -47,6 +51,10 @@ jest.mock("../../../__internal__/utils/logger");
describe("MultiSelect", () => {
let loggerSpy: jest.SpyInstance<void, [message: string]> | jest.Mock;

beforeEach(() => {
mockDOMRect(200, 200, "select-list-scrollable-container");
});

describe("Deprecation warning for uncontrolled", () => {
beforeEach(() => {
loggerSpy = jest.spyOn(Logger, "deprecate");
Expand Down Expand Up @@ -294,7 +302,7 @@ describe("MultiSelect", () => {
simulateDropdownEvent(wrapper, "click");
assertStyleMatch(
{ maxHeight: "120px" },
wrapper.find(StyledSelectListContainer)
wrapper.find(StyledScrollableContainer)
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
exports[`OptionRow renders properly 1`] = `
.c0 {
cursor: pointer;
width: 100%;
position: absolute;
top: 0;
left: 0;
width: 100%;
background-color: var(--colorsUtilityMajor200);
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/select/option-row/option-row.style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ interface StyledOptionRowProps extends Pick<OptionRowProps, "hidden"> {

const StyledOptionRow = styled.tr<StyledOptionRowProps>`
cursor: pointer;
width: 100%;
position: absolute;
top: 0;
left: 0;
width: 100%;
${({ hidden }) => hidden && "display: none;"}
Expand Down
39 changes: 0 additions & 39 deletions src/components/select/select-list/select-list-container.style.ts

This file was deleted.

Loading

0 comments on commit d95770e

Please sign in to comment.