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

feat: add possibility nested dropdown #290

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
38 changes: 38 additions & 0 deletions src/__snapshots__/dropdown.stories.storyshot
Original file line number Diff line number Diff line change
Expand Up @@ -400,5 +400,43 @@ exports[`Storyshots Components/Dropdown Default 1`] = `
</span>
</div>
</div>
<h3
className="sc-dlfnbm dxKCyT"
>
Dropdown with nested dropdowns
</h3>
<div
className="sc-jcVebW kKGLNj sc-dropdown"
>
<div
className="sc-eggNIi gJpqKz trigger"
onBlur={[Function]}
onClick={[Function]}
onFocus={[Function]}
size="base"
tabIndex="0"
>
<span
className="sc-kstrdz gLcDls"
size="base"
>
<i
className="fas fa-star"
/>
</span>
<span
className="sc-hBEYos jaVYGW sc-trigger-text"
>
Help
</span>
<span
className="sc-iBaPrD iJUtiN"
>
<i
className="fas fa-caret-down"
/>
</span>
</div>
</div>
</div>
`;
159 changes: 98 additions & 61 deletions src/lib/components/dropdown/Dropdown.component.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//@flow
import React, { useState, useCallback } from "react";
import React, { useState, useCallback, forwardRef } from "react";
import type { Node } from "react";
import styled, { css } from "styled-components";
import {
Expand All @@ -8,23 +8,45 @@ import {
ButtonText
} from "../button/Button.component";
import * as defaultTheme from "../../style/theme";
import { getThemePropSelector } from "../../utils";
import { getThemePropSelector } from '../../utils';
import { getPositionDropdownMenu } from './utils';

export type Item = {
label: string,
name?: string,
selected?: boolean,
onClick: any => void
onClick?: any => void,
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to type this to SyntheticMouseEvent<HTMLDivElement> (ref : https://flow.org/en/docs/react/events/)

Copy link
Author

Choose a reason for hiding this comment

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

done

submenuIcon?: Node,
submenuItems?: Array<Item>
Copy link
Contributor

Choose a reason for hiding this comment

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

As shown in the design, the documentation and support item also have another external link icon.
Maybe we miss another iconExternal prop?

Copy link
Author

Choose a reason for hiding this comment

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

done

};

type DropdownTriggerContainerProps = {
isItem?: boolean,
dataIndex: number,
open?: boolean,
size?: string,
variant?: string,
title?: string,
onBlur: any => void,
onFocus: any => void,
onClick: any => void,
onMouseEnter: any => void,
onMouseLeave: any => void,
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to type this to SyntheticMouseEvent (ref : https://flow.org/en/docs/react/events/)

Copy link
Author

Choose a reason for hiding this comment

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

done

children: Node,
}

type Items = Array<Item>;
type Props = {
isItem?: boolean,
text?: string,
size?: string,
variant?: string,
title?: string,
items: Items,
icon?: Node,
caret?: boolean
caret?: boolean,
dataIndex?: number,
onClick?: any => void,
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to type this to SyntheticMouseEvent (ref : https://flow.org/en/docs/react/events/)

Copy link
Author

Choose a reason for hiding this comment

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

done

};

const DropdownStyled = styled.div`
Expand All @@ -41,43 +63,19 @@ const DropdownMenuStyled = styled.ul`
position: absolute;
margin: 0;
padding: 0;
border: 1px solid ${getThemePropSelector("primary")};
Copy link
Contributor

Choose a reason for hiding this comment

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

Please double-check with the design, we should have a border for each of the items.

Copy link
Author

Choose a reason for hiding this comment

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

done

z-index: ${defaultTheme.zIndex.dropdown};
max-height: 200px;
min-width: 100%;
overflow: auto;

${props => {
if (
props.size &&
props.triggerSize &&
props.triggerSize.x + props.size.width > window.innerWidth
) {
return css`
right: 0;
top: 100%;
`;
} else if (
props.size &&
props.triggerSize &&
props.triggerSize.y + props.size.height > window.innerHeight
) {
return css`
left: 0;
bottom: ${props.triggerSize.height + "px"};
`;
} else {
return css`
left: 0;
top: 100%;
`;
}

${({ triggerSize, isItem, size, itemIndex = 0, nbItems}) => {
return getPositionDropdownMenu({ isItem, triggerSize, size, nbItems, itemIndex })
}};
`;

const DropdownMenuItemStyled = styled.li`
display: flex;
align-items: center;
justify-content: space-between;
padding: ${defaultTheme.padding.base};
white-space: nowrap;
cursor: pointer;
Expand All @@ -101,86 +99,125 @@ const Caret = styled.span`

const TriggerStyled = ButtonStyled.withComponent("div");

const DropdownTriggerContainer = forwardRef<DropdownTriggerContainerProps, Element>(({isItem, dataIndex, open, size , variant, title, onBlur, onFocus, onClick, onMouseEnter, onMouseLeave, children, ...rest}, ref) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering do you have the prettier extension installed? Please enable it to make sure the code follows the .prettierrc config.
Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

done

return isItem ? (
<DropdownMenuItemStyled
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
data-index={dataIndex}
onClick={onClick}
ref={ref}
>
{children}
</DropdownMenuItemStyled>
) : (
<DropdownStyled
active={open}
variant={variant}
className="sc-dropdown"
{...rest}
>
<TriggerStyled
variant={variant}
size={size}
className="trigger"
onBlur={onBlur}
onFocus={onFocus}
onClick={onClick}
tabIndex="0"
title={title}
ref={ref}
>
{children}
</TriggerStyled>
</DropdownStyled>
)
})

function Dropdown({
isItem = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think separating exposed Dropdown component and an internal non-exposed DropdownAsAnItem component would be a better design. It would for example avoid confusion of having an isItem or dataIndex(which are implementation details) props available when trying to use the Dropdown component. I think it is very important to keep our exposed API very clear and not add props linked to the implementation in what we expose.

Copy link
Author

Choose a reason for hiding this comment

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

done

items,
text,
icon,
size = "base",
variant = "base",
title,
caret = true,
dataIndex = null,
onClick = null,
...rest
}: Props) {
const [open, setOpen] = useState(false);
const [menuSize, setMenuSize] = useState();
const [triggerSize, setTriggerSize] = useState();
const [itemIndex, setItemIndex] = useState()

const refMenuCallback = useCallback(node => {
if (node !== null) {
setMenuSize(node.getBoundingClientRect());
}
}, []);
}, [setMenuSize]);

const refTriggerCallback = useCallback(node => {
if (node !== null) {
setTriggerSize(node.getBoundingClientRect());
}
}, []);
}, [setTriggerSize]);

return (
<DropdownStyled
active={open}
variant={variant}
className="sc-dropdown"
{...rest}
>
<TriggerStyled
<DropdownTriggerContainer
open={open}
isItem={isItem}
onMouseEnter={(e) => {
setItemIndex(e && e.target && e.target.getAttribute('data-index') || 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

We got a flow error. https://eslint.org/docs/rules/no-mixed-operators
setItemIndex(e && e.target && e.target.getAttribute('data-index')) || 0

Copy link
Author

Choose a reason for hiding this comment

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

done

setOpen(true)
}}
onMouseLeave={() => setOpen(false)}
variant={variant}
size={size}
className="trigger"
onBlur={() => setOpen(!open)}
onFocus={() => setOpen(!open)}
onClick={event => event.stopPropagation()}
tabIndex="0"
onClick={onClick ? onClick : event => event.stopPropagation()}
title={title}
ref={refTriggerCallback}
dataIndex={dataIndex}
{...rest}
>
{icon && (
<ButtonIcon text={text} size={size}>
{icon}
</ButtonIcon>
)}
{text && <ButtonText className="sc-trigger-text">{text}</ButtonText>}
{caret && (
{caret && items.length > 0 && (
<Caret>
<i className="fas fa-caret-down" />
<i className={`fas fa-caret-${isItem ? 'right' : 'down'}`} />
Copy link
Contributor

Choose a reason for hiding this comment

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

The right arrow icon should be chevron instead of caret
<i className="fas fa-chevron-right" />

Copy link
Author

Choose a reason for hiding this comment

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

done

</Caret>
)}
{open && (
<DropdownMenuStyled
className="menu-item"
postion={"right"}
ref={refMenuCallback}
size={menuSize}
triggerSize={triggerSize}
itemIndex={itemIndex}
nbItems={items.length}
isItem={isItem}
>
{items.map(({ label, onClick, ...itemRest }) => {
return (
<DropdownMenuItemStyled
className="menu-item-label"
key={label}
onClick={onClick}
variant={variant}
{...itemRest}
>
{label}
</DropdownMenuItemStyled>
);
{items.map(({ label, onClick, submenuIcon = null, submenuItems = [], ...itemRest }, index) => {
return <Dropdown
isItem
key={label}
text={label}
icon={submenuIcon}
items={submenuItems}
dataIndex={index}
onClick={onClick}
/>
})}
</DropdownMenuStyled>
)}
</TriggerStyled>
</DropdownStyled>
</DropdownTriggerContainer>
);
}

Expand Down
72 changes: 72 additions & 0 deletions src/lib/components/dropdown/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { css } from 'styled-components';

const getBorderCollisionDetection = (isItem, triggerSize, size) => {
let isTopHit = false
let isRightHit = false
let isBottomHit = false
let isLeftHit = false

if (size && triggerSize && triggerSize.top - size.height <= 0) {
isTopHit = true
}
if (size && triggerSize && triggerSize.right + triggerSize.width > window.innerWidth) {
isRightHit = true
}
if (size && triggerSize && triggerSize.top + size.height >= window.innerHeight) {
isBottomHit = true
}
if (size && triggerSize && triggerSize.left - triggerSize.width <= 0) {
isLeftHit = true
}

return {
isTopHit,
isRightHit,
isBottomHit,
isLeftHit
}
}

export const getPositionDropdownMenu = ({ isItem, triggerSize, size, nbItems, itemIndex }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should be unit tested

Copy link
Author

Choose a reason for hiding this comment

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

done

const { isTopHit, isRightHit, isBottomHit, isLeftHit } = getBorderCollisionDetection(isItem, triggerSize, size)

if (isItem) {
// Check collision for dropdown acting as an item
if (isRightHit && isBottomHit) {
return css`
right: ${size.width}px;
bottom: ${-triggerSize.height * (itemIndex - nbItems + 1)}px;
Copy link
Contributor

Choose a reason for hiding this comment

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

What if some items takes more than one line ?

Copy link
Author

@romainlancia romainlancia Apr 8, 2021

Choose a reason for hiding this comment

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

@JBWatenbergScality Trying to find a solution but this one is a bit tricky to handle because indeed if an item have a height superior to other item, the new menu generated will display with a wrong offset. The problem I have to get each item size of the previous menu to place the new menu correctly but it will add a lot of complexity to the code. I can figure out a way to implement that in a simple way, do you have any suggestions ?

`;
} else if (isLeftHit && isBottomHit || isBottomHit) {
return css`
left: ${size.width}px;
bottom: ${-triggerSize.height * (itemIndex - nbItems + 1)}px;
`;
} else if (isTopHit && isRightHit || isRightHit) {
return css`
right: ${size.width}px;
top: ${triggerSize.height * itemIndex}px;
`;
}
else {
return css`
left: ${triggerSize.width}px;
top: ${triggerSize.height * itemIndex}px;
`;
}
}
else {
// Check collision for dropdown acting as a root button*
if (isBottomHit) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Please format this using prettier

Copy link
Author

Choose a reason for hiding this comment

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

done

return css`
left: 0;
bottom: ${triggerSize.height}px;
`;
} else {
return css`
left: 0;
top: 100%;
`;
}
}
}
Loading