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

HELP-8042 slowness due to floating UI #334

Merged
merged 5 commits into from
Sep 4, 2024
Merged
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
104 changes: 71 additions & 33 deletions lib/components/ActionsMenu/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@
opacity: 0;
visibility: hidden;

&.hack-for-legacy-tests {
position: absolute;
pointer-events: none;
opacity: 0;
visibility: hidden;
height: 0;
width: 0;
padding: 0;
}

&.visible {
visibility: visible;
opacity: 1;
Expand Down Expand Up @@ -269,7 +279,7 @@
onTriggerFocus,
closeMenu,
closeOnClick = false,
"data-testid": dataTestId = "ActionsMenu",

Check warning on line 282 in lib/components/ActionsMenu/index.js

View workflow job for this annotation

GitHub Actions / build

'data-testid' is missing in props validation
...props
}) => {
const id = useId();
Expand All @@ -288,19 +298,31 @@
const triggerRef = useMergeRefs([actionMenu.refs.setReference, childrenRef]);
const ref = useMergeRefs([actionMenu.refs.setFloating]);

const triggerProps = {
ariaLabel,
"aria-label": ariaLabel,
onFocus: onTriggerFocus,
id,
...actionMenu.getReferenceProps({
...props,
onClick: onToggle,
ref: triggerRef,
"data-state": actionMenu.open ? "open" : "closed",
"data-testid": dataTestId
})
};
const triggerProps = useMemo(
() => ({
ariaLabel,
"aria-label": ariaLabel,
onFocus: onTriggerFocus,
id,
...actionMenu.getReferenceProps({
...props,
onClick: onToggle,
ref: triggerRef,
"data-state": actionMenu.open ? "open" : "closed",
"data-testid": dataTestId
})
}),
[
ariaLabel,
onTriggerFocus,
id,
actionMenu,
onToggle,
props,
triggerRef,
dataTestId
]
);

let triggerComponent = (
<Control {...triggerProps}>
Expand Down Expand Up @@ -328,25 +350,21 @@
[closeOnClick, closeMenu, id, actionMenu]
);

const visible = actionMenu.context.open;
const style = useMemo(
() => ({
...actionMenu.floatingStyles,
zIndex: getFloatingUiZIndex(actionMenu.refs.reference)
}),
[actionMenu.floatingStyles, actionMenu.refs.reference]
);

const ActionMenuContent = (
<StyledActionsMenuContainer
ref={ref}
style={{
...actionMenu.floatingStyles,
zIndex: getFloatingUiZIndex(actionMenu.refs.reference)
}}
aria-labelledby={actionMenu.labelId}
{...actionMenu.getFloatingProps(props)}
className={`actionMenu-content ${visible ? "visible" : ""}`}
aria-hidden={visible ? "false" : "true"}
data-testid={`${dataTestId}__menu`}
>
<Menu menuWidth={menuWidth} isOpen={toggleState}>
{children}
</Menu>
</StyledActionsMenuContainer>
const menuDataTestId = useMemo(() => `${dataTestId}__menu`, [dataTestId]);

const { getFloatingProps } = actionMenu;

const floatingProps = useMemo(
() => getFloatingProps(props),
[getFloatingProps, props]
);

const component = (
Expand All @@ -359,7 +377,19 @@
root={getFloatingUiRootElement(actionMenu.refs.reference)}
>
<FloatingFocusManager context={actionMenu.context} modal={true}>
{ActionMenuContent}
<StyledActionsMenuContainer
aria-labelledby={actionMenu.labelId}
data-testid={menuDataTestId}
{...floatingProps}
style={style}
className="actionMenu-content visible"
aria-hidden="false"
ref={ref}
>
<Menu menuWidth={menuWidth} isOpen={toggleState}>
{children}
</Menu>
</StyledActionsMenuContainer>
</FloatingFocusManager>
</FloatingPortal>
) : (
Expand All @@ -369,7 +399,15 @@
but in a hidden state ensures that tests pass. * Ideally, we would
update all the tests in teamform-app-ui to open the ActionsMenu *
before assertion. **/
ActionMenuContent
<StyledActionsMenuContainer
aria-labelledby={actionMenu.labelId}
data-testid={menuDataTestId}
className="actionMenu-content hack-for-legacy-tests"
>
<Menu menuWidth={menuWidth} isOpen={toggleState}>
{children}
</Menu>
</StyledActionsMenuContainer>
)}
</Wrapper>
</ActionMenuContext.Provider>
Expand Down
104 changes: 71 additions & 33 deletions lib/components/Popover/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from "react";
import React, { useState, useMemo } from "react";
import {
useFloating,
autoUpdate,
Expand Down Expand Up @@ -79,9 +79,15 @@ const StyledPopover = styled.div`
box-shadow: ${themeGet("shadows.boxDefault")};
user-select: ${(props) => (props.enableSelectAll ? "all" : "auto")};

pointer-events: none;
opacity: 0;
visibility: hidden;
&.hack-for-legacy-tests {
position: absolute;
pointer-events: none;
opacity: 0;
visibility: hidden;
height: 0;
width: 0;
padding: 0;
}

&.visible {
opacity: 1;
Expand Down Expand Up @@ -312,37 +318,42 @@ export default function Popover({
role
]);

const triggerProps = {
...getReferenceProps({ ref: refs.setReference }),
tabIndex: "0"
};
const triggerProps = useMemo(
() => ({
...getReferenceProps({ ref: refs.setReference }),
tabIndex: "0"
}),
[getReferenceProps, refs.setReference]
);

const directionClass =
context.placement === DIRECTIONS_MAP[direction]
? direction
: context.placement;
const directionClass = useMemo(
() =>
context.placement === DIRECTIONS_MAP[direction]
? direction
: context.placement,
[context.placement, direction]
);

const Popover = (
<StyledPopover
textAlign={textAlign}
width={width}
enableSelectAll={enableSelectAll}
className={`Tooltip popover ${
visible ? "visible" : ""
} ${directionClass}`}
ref={refs.setFloating}
style={{
...floatingStyles,
zIndex: getFloatingUiZIndex(context.refs.reference)
}}
{...getFloatingProps()}
ariaLabel={ariaLabel}
>
{text}
</StyledPopover>
const style = useMemo(
() => ({
...floatingStyles,
zIndex: getFloatingUiZIndex(context.refs.reference)
}),
[floatingStyles, context.refs.reference]
);

const containsLinks = refs.floating?.current?.querySelectorAll("a").length;

const visiblePopoverClassName = useMemo(
() => `Tooltip popover visible ${directionClass}`,
[directionClass]
);

const floatingProps = useMemo(
() => getFloatingProps(props),
[getFloatingProps, props]
);

return (
<Container
inlineBlock={inlineBlock}
Expand Down Expand Up @@ -374,21 +385,48 @@ export default function Popover({
isRenderedInReactSelectMenu(context.refs.reference) && -1
}
>
{Popover}
<StyledPopover
className={visiblePopoverClassName}
ref={refs.setFloating}
textAlign={textAlign}
width={width}
enableSelectAll={enableSelectAll}
ariaLabel={ariaLabel}
style={style}
{...floatingProps}
>
{text}
</StyledPopover>
</FloatingFocusManager>
) : (
Popover
<StyledPopover
className={visiblePopoverClassName}
ref={refs.setFloating}
textAlign={textAlign}
width={width}
enableSelectAll={enableSelectAll}
ariaLabel={ariaLabel}
style={style}
{...floatingProps}
>
{text}
</StyledPopover>
)}
</FloatingPortal>
) : (
Popover
/*
* HACK: Fixing all the broken tests in teamform-app-ui is too time consuming
* right this moment with a lot of the tests asserting against Popover items.
* Rendering the markup even when closed but in a hidden state ensures that tests pass.
* Ideally, we would update all the tests in teamform-app-ui to open the Popover
* before assertion.
**/
<StyledPopover
ariaLabel={ariaLabel}
className="Tooltip popover hack-for-legacy-tests"
>
{text}
</StyledPopover>
))}

{children}
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "orcs-design-system",
"version": "3.2.36",
"version": "3.2.37",
"engines": {
"node": "20.12.2"
},
Expand Down
Loading