Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(select): ensure select-list has correct box-shadow when placement is top #7138

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,11 @@ const SelectList = React.forwardRef(
) => {
const [currentOptionsListIndex, setCurrentOptionsListIndex] = useState(-1);
const [scrollbarWidth, setScrollbarWidth] = useState(0);
const [actualPlacement, setActualPlacement] = useState(listPlacement);
const lastFilter = useRef("");
const listRef = useRef(null);
const tableRef = useRef<HTMLTableSectionElement>(null);
const listWrapperRef = useRef<HTMLDivElement>(null);
const listActionButtonRef = useRef<HTMLButtonElement>(null);
const { blockScroll, allowScroll } = useScrollBlock();
const actionButtonHeight = useRef(0);
Expand Down Expand Up @@ -648,6 +650,25 @@ const SelectList = React.forwardRef(
[listWidth, flipEnabled],
);

// set the placement of the list based on the floating placement
const setPlacement = useCallback(() => {
if (isOpen) {
const floatingPlacement = listWrapperRef.current?.getAttribute(
"data-floating-placement",
) as ListPlacement;

setActualPlacement(floatingPlacement);
}
}, [isOpen]);

useEffect(() => {
setPlacement();
window.addEventListener("resize", setPlacement);
return () => {
window.removeEventListener("resize", setPlacement);
};
}, [setPlacement]);

const loader = isLoading ? (
<StyledSelectLoaderContainer key="loader">
<Loader />
Expand Down Expand Up @@ -705,9 +726,11 @@ const SelectList = React.forwardRef(
animationFrame
>
<StyledSelectListContainer
ref={listWrapperRef}
data-element="select-list-wrapper"
data-role="select-list-wrapper"
isLoading={isLoading}
placement={actualPlacement}
{...listProps}
>
<StyledScrollableContainer
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import styled, { css } from "styled-components";
import { baseTheme } from "../../../../style/themes";
import { SelectListProps } from ".";

interface StyledSelectListProps {
listHeight?: number;
Expand Down Expand Up @@ -101,11 +100,17 @@ const StyledSelectListTableBody = styled.tbody<StyledSelectListProps>`
height: ${({ listHeight }) => `${listHeight}px`};
`;

const StyledSelectListContainer = styled.div<
Pick<SelectListProps, "isLoading">
>`
interface StyledSelectListContainerProps {
isLoading?: boolean;
placement?: string;
}

const StyledSelectListContainer = styled.div<StyledSelectListContainerProps>`
background-color: white;
box-shadow: var(--boxShadow100);
box-shadow: ${({ placement }) =>
placement === "top"
? "0 -5px 5px 0 #00141e33, 0 -10px 10px 0 #00141e1a"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no design token for this so I just negated the y-offset of the original token value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any design tokens that could be used instead of those hex colours?

: "var(--boxShadow100)"};
animation: fadeIn 250ms ease-out;
position: absolute;
z-index: ${({ theme }) => theme.zIndex.popover};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,27 +55,26 @@ Default.storyName = "Default";

export const ListPlacement: Story = () => {
return (
<Box height={220} alignContent="end">
<FilterableSelect
name="simple"
id="simple"
label="color"
labelInline
listPlacement="top"
>
<Option text="Amber" value="1" />
<Option text="Black" value="2" />
<Option text="Blue" value="3" />
<Option text="Brown" value="4" />
<Option text="Green" value="5" />
<Option text="Orange" value="6" />
<Option text="Pink" value="7" />
<Option text="Purple" value="8" />
<Option text="Red" value="9" />
<Option text="White" value="10" />
<Option text="Yellow" value="11" />
</FilterableSelect>
</Box>
<FilterableSelect
name="simple"
id="simple"
label="color"
labelInline
listPlacement="top"
mt={200}
>
<Option text="Amber" value="1" />
<Option text="Black" value="2" />
<Option text="Blue" value="3" />
<Option text="Brown" value="4" />
<Option text="Green" value="5" />
<Option text="Orange" value="6" />
<Option text="Pink" value="7" />
<Option text="Purple" value="8" />
<Option text="Red" value="9" />
<Option text="White" value="10" />
<Option text="Yellow" value="11" />
</FilterableSelect>
);
};
ListPlacement.storyName = "List Placement";
Expand Down
41 changes: 20 additions & 21 deletions src/components/select/multi-select/multi-select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,26 @@ Default.storyName = "Default";

export const ListPlacement: Story = () => {
return (
<Box height={200} alignContent="end">
<MultiSelect
name="simple"
id="simple"
label="color"
labelInline
listPlacement="top"
>
<Option text="Amber" value="1" />
<Option text="Black" value="2" />
<Option text="Blue" value="3" />
<Option text="Brown" value="4" />
<Option text="Green" value="5" />
<Option text="Orange" value="6" />
<Option text="Pink" value="7" />
<Option text="Purple" value="8" />
<Option text="Red" value="9" />
<Option text="White" value="10" />
<Option text="Yellow" value="11" />
</MultiSelect>
</Box>
<MultiSelect
name="simple"
id="simple"
label="color"
labelInline
listPlacement="top"
mt={200}
>
<Option text="Amber" value="1" />
<Option text="Black" value="2" />
<Option text="Blue" value="3" />
<Option text="Brown" value="4" />
<Option text="Green" value="5" />
<Option text="Orange" value="6" />
<Option text="Pink" value="7" />
<Option text="Purple" value="8" />
<Option text="Red" value="9" />
<Option text="White" value="10" />
<Option text="Yellow" value="11" />
</MultiSelect>
);
};
ListPlacement.storyName = "List Placement";
Expand Down
46 changes: 46 additions & 0 deletions src/components/select/simple-select/simple-select.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,52 @@ test.describe("SimpleSelect component", () => {
});
});

(
[
[
"top",
"rgba(0, 20, 30, 0.2) 0px -5px 5px 0px, rgba(0, 20, 30, 0.1) 0px -10px 10px 0px",
],
[
"bottom",
"rgba(0, 20, 30, 0.2) 0px 5px 5px 0px, rgba(0, 20, 30, 0.1) 0px 10px 10px 0px",
],
] as [NonNullable<SimpleSelectProps["listPlacement"]>, string][]
).forEach(([position, boxShadow]) => {
test(`should render list with expected box-shadow when listPosition is ${position}`, async ({
mount,
page,
}) => {
await mount(
<SimpleSelectComponent listPlacement={position} flipEnabled={false} />,
);

await selectText(page).click();
const listElement = selectListPosition(page);
await expect(listElement).toHaveCSS("box-shadow", boxShadow);
await expect(listElement).toBeVisible();
});
});

test("should update box-shadow when placement changes due to window resize", async ({
mount,
page,
}) => {
await mount(<SimpleSelectComponent mt={200} />);

await selectText(page).click();
const listElement = selectListPosition(page);
await expect(listElement).toHaveCSS(
"box-shadow",
"rgba(0, 20, 30, 0.2) 0px 5px 5px 0px, rgba(0, 20, 30, 0.1) 0px 10px 10px 0px",
);
await page.setViewportSize({ width: 1200, height: 250 });
await expect(listElement).toHaveCSS(
"box-shadow",
"rgba(0, 20, 30, 0.2) 0px -5px 5px 0px, rgba(0, 20, 30, 0.1) 0px -10px 10px 0px",
);
});

test("should have correct hover state of list option", async ({
mount,
page,
Expand Down
41 changes: 20 additions & 21 deletions src/components/select/simple-select/simple-select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,27 +100,26 @@ IsOptional.storyName = "IsOptional";

export const ListPlacement: Story = () => {
return (
<Box height={200} alignContent="end">
<Select
name="simple"
id="simple"
label="color"
labelInline
listPlacement="top"
>
<Option text="Amber" value="1" />
<Option text="Black" value="2" />
<Option text="Blue" value="3" />
<Option text="Brown" value="4" />
<Option text="Green" value="5" />
<Option text="Orange" value="6" />
<Option text="Pink" value="7" />
<Option text="Purple" value="8" />
<Option text="Red" value="9" />
<Option text="White" value="10" />
<Option text="Yellow" value="11" />
</Select>
</Box>
<Select
name="simple"
id="simple"
label="color"
labelInline
listPlacement="top"
mt={200}
>
<Option text="Amber" value="1" />
<Option text="Black" value="2" />
<Option text="Blue" value="3" />
<Option text="Brown" value="4" />
<Option text="Green" value="5" />
<Option text="Orange" value="6" />
<Option text="Pink" value="7" />
<Option text="Purple" value="8" />
<Option text="Red" value="9" />
<Option text="White" value="10" />
<Option text="Yellow" value="11" />
</Select>
);
};
ListPlacement.storyName = "List Placement";
Expand Down
Loading