From 72c506fa23e5b33bacdffc75f6d78d151d19bd17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Tranta?= Date: Fri, 7 Feb 2025 11:32:41 +0100 Subject: [PATCH] FIX: flicking device status (#16873) --- packages/components/__mocks__/fileMock.js | 1 + packages/components/jest.config.js | 4 + packages/components/package.json | 1 + .../components/src/components/Icon/Icon.tsx | 2 +- .../components/Tooltip/Tooltip.stories.tsx | 5 +- .../src/components/Tooltip/Tooltip.test.tsx | 99 +++++++++++++++++++ .../src/components/Tooltip/Tooltip.tsx | 35 +++++-- .../components/Tooltip/TooltipFloatingUi.tsx | 5 +- .../TruncateWithTooltip.stories.tsx | 1 + .../TruncateWithTooltip.tsx | 47 +++++---- .../components/src/utils/frameProps.test.ts | 59 +++++++++++ packages/components/src/utils/frameProps.tsx | 24 ++--- .../src/utils/transientProps.test.ts | 28 ++++++ .../DeviceSelector/DeviceSelector.tsx | 10 +- .../suite/src/views/dashboard/PromoBanner.tsx | 2 +- .../SettingsDevice/FirmwareVersion.tsx | 5 +- yarn.lock | 10 ++ 17 files changed, 280 insertions(+), 58 deletions(-) create mode 100644 packages/components/__mocks__/fileMock.js create mode 100644 packages/components/src/components/Tooltip/Tooltip.test.tsx create mode 100644 packages/components/src/utils/frameProps.test.ts create mode 100644 packages/components/src/utils/transientProps.test.ts diff --git a/packages/components/__mocks__/fileMock.js b/packages/components/__mocks__/fileMock.js new file mode 100644 index 00000000000..86059f36292 --- /dev/null +++ b/packages/components/__mocks__/fileMock.js @@ -0,0 +1 @@ +module.exports = 'test-file-stub'; diff --git a/packages/components/jest.config.js b/packages/components/jest.config.js index 5198206cc9d..1ae6676036f 100644 --- a/packages/components/jest.config.js +++ b/packages/components/jest.config.js @@ -4,4 +4,8 @@ module.exports = { ...baseConfig, setupFilesAfterEnv: ['/jest.setup.js'], testEnvironment: 'jsdom', + moduleNameMapper: { + ...baseConfig.moduleNameMapper, + '\\.svg$': '/__mocks__/fileMock.js', + }, }; diff --git a/packages/components/package.json b/packages/components/package.json index 9bd514a173b..a9204f39f8f 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -59,6 +59,7 @@ "@storybook/react-webpack5": "^7.6.13", "@storybook/theming": "^7.6.13", "@testing-library/react": "14.2.1", + "@testing-library/user-event": "^14.6.1", "@trezor/eslint": "workspace:*", "@types/react": "18.2.79", "@types/react-date-range": "^1.4.9", diff --git a/packages/components/src/components/Icon/Icon.tsx b/packages/components/src/components/Icon/Icon.tsx index dd1f6a44e9f..a2c8466b1fe 100644 --- a/packages/components/src/components/Icon/Icon.tsx +++ b/packages/components/src/components/Icon/Icon.tsx @@ -185,8 +185,8 @@ export const Icon = forwardRef( onClick={onClick ? handleClick : undefined} className={className} ref={ref} - $cursor={cursor ?? (onClick ? 'pointer' : undefined)} {...frameProps} + $cursor={cursor ?? (onClick ? 'pointer' : undefined)} > = { offset: 10, delayHide: TOOLTIP_DELAY_SHORT, delayShow: TOOLTIP_DELAY_SHORT, + ...getFramePropsStory(allowedTooltipFrameProps).args, }, argTypes: { hasArrow: { @@ -176,5 +178,6 @@ export const Tooltip: StoryObj = { control: 'select', options: DELAYS, }, + ...getFramePropsStory(allowedTooltipFrameProps).argTypes, }, }; diff --git a/packages/components/src/components/Tooltip/Tooltip.test.tsx b/packages/components/src/components/Tooltip/Tooltip.test.tsx new file mode 100644 index 00000000000..c40c3a44f3c --- /dev/null +++ b/packages/components/src/components/Tooltip/Tooltip.test.tsx @@ -0,0 +1,99 @@ +import { act, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { Tooltip } from './Tooltip'; + +describe('Tooltip', () => { + it('should show tooltip on hover when isActive is true', async () => { + const tooltipContent = 'Tooltip Content'; + render( + + + , + ); + + const trigger = screen.getByText('Hover me'); + await userEvent.hover(trigger); + + const tooltip = screen.getByText(tooltipContent); + expect(tooltip).toBeInTheDocument(); + }); + + it('should show tooltip on hover when isActive is not defined (default behavior)', async () => { + const tooltipContent = 'Tooltip Content'; + render( + + + , + ); + await act(() => {}); + + const trigger = screen.getByText('Hover me'); + + await userEvent.hover(trigger); + + const tooltip = screen.getByText(tooltipContent); + expect(tooltip).toBeInTheDocument(); + }); + + it('should not show tooltip on hover when isActive is false', async () => { + const tooltipContent = 'Tooltip Content'; + render( + + + , + ); + + const trigger = screen.getByText('Hover me'); + await userEvent.hover(trigger); + + const tooltip = screen.queryByText(tooltipContent); + expect(tooltip).not.toBeInTheDocument(); + }); + + it('should hide tooltip when mouse leaves trigger element', async () => { + const tooltipContent = 'Tooltip Content'; + render( + + + , + ); + + const trigger = screen.getByText('Hover me'); + + await userEvent.hover(trigger); + + const currentTrigger = screen.getByText('Hover me'); + expect(currentTrigger.parentElement).toHaveAttribute('data-state', 'open'); + + await userEvent.unhover(trigger); + + const triggerBefore = screen.getByText('Hover me'); + + // NOTE: for some reason, the content is still in the DOM but the state is definitely closed + const parent = triggerBefore.parentElement; + expect(parent).toHaveAttribute('data-state', 'closed'); + }); + + it('should apply the cursor prop to the content wrapper', () => { + const tooltipContent = 'Tooltip Content'; + render( + + + , + ); + + expect(screen.getByText('Hover me').parentElement).toHaveStyle({ cursor: 'pointer' }); + }); + + it('should should apply the default=help cursor when the passed cursor is undefined', () => { + const tooltipContent = 'Tooltip Content'; + render( + + + , + ); + + expect(screen.getByText('Hover me').parentElement).toHaveStyle({ cursor: 'help' }); + }); +}); diff --git a/packages/components/src/components/Tooltip/Tooltip.tsx b/packages/components/src/components/Tooltip/Tooltip.tsx index 5371076984e..127e9b5f246 100644 --- a/packages/components/src/components/Tooltip/Tooltip.tsx +++ b/packages/components/src/components/Tooltip/Tooltip.tsx @@ -11,15 +11,27 @@ import { TooltipBox, TooltipBoxProps } from './TooltipBox'; import { TOOLTIP_DELAY_SHORT, TooltipDelay } from './TooltipDelay'; import { TooltipContent, TooltipFloatingUi, TooltipTrigger } from './TooltipFloatingUi'; import { intermediaryTheme } from '../../config/colors'; +import { + FrameProps, + FramePropsKeys, + pickAndPrepareFrameProps, + withFrameProps, +} from '../../utils/frameProps'; +import { TransientProps } from '../../utils/transientProps'; import { Icon } from '../Icon/Icon'; -export type Cursor = 'inherit' | 'pointer' | 'help' | 'default' | 'not-allowed'; +export type TooltipInteraction = 'none' | 'hover'; + +export const allowedTooltipFrameProps = ['cursor'] as const satisfies FramePropsKeys[]; +export type AllowedFrameProps = Pick; const Wrapper = styled.div<{ $isFullWidth: boolean }>` width: ${({ $isFullWidth }) => ($isFullWidth ? '100%' : 'auto')}; `; -const Content = styled.div<{ $dashed: boolean; $isInline: boolean; $cursor: Cursor }>` +const Content = styled.div< + { $dashed: boolean; $isInline: boolean } & TransientProps +>` display: ${({ $isInline }) => ($isInline ? 'inline-flex' : 'flex')}; align-items: center; justify-content: flex-start; @@ -27,9 +39,9 @@ const Content = styled.div<{ $dashed: boolean; $isInline: boolean; $cursor: Curs cursor: ${({ $cursor }) => $cursor}; border-bottom: ${({ $dashed, theme }) => $dashed && `1.5px dotted ${transparentize(0.66, theme.textSubdued)}`}; -`; -export type TooltipInteraction = 'none' | 'hover'; + ${withFrameProps} +`; type ManagedModeProps = { isOpen?: boolean; @@ -46,13 +58,12 @@ type UnmanagedModeProps = { }; type TooltipUiProps = { + isActive?: boolean; children: ReactNode; className?: string; - disabled?: boolean; dashed?: boolean; offset?: number; shift?: ShiftOptions; - cursor?: Cursor; isFullWidth?: boolean; placement?: Placement; hasArrow?: boolean; @@ -60,13 +71,14 @@ type TooltipUiProps = { appendTo?: HTMLElement | null | MutableRefObject; zIndex?: ZIndexValues; isInline?: boolean; -}; +} & AllowedFrameProps; export type TooltipProps = (ManagedModeProps | UnmanagedModeProps) & TooltipUiProps & TooltipBoxProps; export const Tooltip = ({ + isActive = true, placement = 'top', children, isLarge = false, @@ -75,12 +87,10 @@ export const Tooltip = ({ delayHide = TOOLTIP_DELAY_SHORT, maxWidth = 400, offset = spacings.sm, - cursor = 'help', content, addon, title, headerIcon, - disabled, className, isFullWidth = false, isInline = false, @@ -90,7 +100,10 @@ export const Tooltip = ({ appendTo, shift, zIndex = zIndices.tooltip, + ...rest }: TooltipProps) => { + const frameProps = pickAndPrepareFrameProps(rest, allowedTooltipFrameProps); + if (!content || !children) { return <>{children}; } @@ -101,6 +114,7 @@ export const Tooltip = ({ return ( {children} {hasIcon && } diff --git a/packages/components/src/components/Tooltip/TooltipFloatingUi.tsx b/packages/components/src/components/Tooltip/TooltipFloatingUi.tsx index 2ff085fcc02..6c79e674b98 100644 --- a/packages/components/src/components/Tooltip/TooltipFloatingUi.tsx +++ b/packages/components/src/components/Tooltip/TooltipFloatingUi.tsx @@ -46,6 +46,7 @@ type Delay = { }; interface TooltipOptions { + isActive?: boolean; // Determines if the tooltip is active - reacts to the hovering isInitiallyOpen?: boolean; placement?: Placement; isOpen?: boolean; @@ -62,6 +63,7 @@ type UseTooltipReturn = ReturnType & { } & UseFloatingReturn; export const useTooltip = ({ + isActive = true, isInitiallyOpen = false, placement = 'top', isOpen: isControlledOpen, @@ -73,7 +75,8 @@ export const useTooltip = ({ const arrowRef = useRef(null); const [isUncontrolledTooltipOpen, setIsUncontrolledTooltipOpen] = useState(isInitiallyOpen); - const open = isControlledOpen ?? isUncontrolledTooltipOpen; + // NOTE: if the tooltip is overall inactive (isActive === false), always hide it / never display it + const open = isActive === false ? false : isControlledOpen ?? isUncontrolledTooltipOpen; const setOpen = setControlledOpen ?? setIsUncontrolledTooltipOpen; const data = useFloating({ diff --git a/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.stories.tsx b/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.stories.tsx index 18c99733291..609fc790286 100644 --- a/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.stories.tsx +++ b/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.stories.tsx @@ -9,6 +9,7 @@ import { const Container = styled.div` overflow: hidden; white-space: nowrap; + max-width: 200px; `; const meta: Meta = { diff --git a/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.tsx b/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.tsx index b55e55f523d..bfb2ca475ff 100644 --- a/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.tsx +++ b/packages/components/src/components/typography/TruncateWithTooltip/TruncateWithTooltip.tsx @@ -2,7 +2,7 @@ import { useEffect, useRef, useState } from 'react'; import styled from 'styled-components'; -import { Cursor, Tooltip } from '../../Tooltip/Tooltip'; +import { Tooltip, type AllowedFrameProps as TooltipAllowedFrameProps } from '../../Tooltip/Tooltip'; import { TooltipDelay } from '../../Tooltip/TooltipDelay'; const EllipsisContainer = styled.div` @@ -10,52 +10,49 @@ const EllipsisContainer = styled.div` overflow: hidden; `; -export interface TruncateWithTooltipProps { +export interface TruncateWithTooltipProps extends TooltipAllowedFrameProps { children: React.ReactNode; delayShow?: TooltipDelay; - cursor?: Cursor; } -export const TruncateWithTooltip = ({ - children, - delayShow, - cursor = 'inherit', -}: TruncateWithTooltipProps) => { - const [isTooltipVisible, setIsTooltipVisible] = useState(false); +export const TruncateWithTooltip = ({ children, delayShow, ...rest }: TruncateWithTooltipProps) => { + const [isEllipsisActive, setEllipsisActive] = useState(false); const containerRef = useRef(null); - const scrollWidth = containerRef.current?.scrollWidth ?? null; - const scrollHeight = containerRef.current?.scrollHeight ?? null; - useEffect(() => { - if (!containerRef.current || !scrollWidth || !scrollHeight) return; + if (!containerRef.current) return; + const resizeObserver = new ResizeObserver(entries => { + const scrollWidth = containerRef.current?.scrollWidth ?? null; + const scrollHeight = containerRef.current?.scrollHeight ?? null; const borderBoxSize = entries[0].borderBoxSize?.[0]; - if (!borderBoxSize) { + if (!borderBoxSize || !scrollWidth || !scrollHeight) { return; } const { inlineSize: elementWidth, blockSize: elementHeight } = borderBoxSize; - setIsTooltipVisible( - scrollWidth > Math.ceil(elementWidth) || scrollHeight > Math.ceil(elementHeight), - ); + const nextEllipsisActive = + scrollWidth > Math.ceil(elementWidth) || scrollHeight > Math.ceil(elementHeight); + + setEllipsisActive(nextEllipsisActive); }); resizeObserver.observe(containerRef.current); return () => resizeObserver.disconnect(); - }, [children, scrollWidth, scrollHeight]); + }, [children]); return ( - {isTooltipVisible ? ( - - {children} - - ) : ( - children - )} + + {isEllipsisActive ? {children} : children} + ); }; diff --git a/packages/components/src/utils/frameProps.test.ts b/packages/components/src/utils/frameProps.test.ts new file mode 100644 index 00000000000..13011e201ba --- /dev/null +++ b/packages/components/src/utils/frameProps.test.ts @@ -0,0 +1,59 @@ +import { makePropsTransient } from './transientProps'; + +describe('frameprops', () => { + describe('type tests', () => { + it('should return the same keys just with the $ prefix as type', () => { + const result: { + $a: 1; + $b: 2; + } = makePropsTransient({ a: 1, b: 2 } as const); + expect(result).toEqual({ + $a: 1, + $b: 2, + }); + }); + + it('should correctly be able to pick individual key', () => { + const result: { + $a: 1; + $b: 2; + } = makePropsTransient({ a: 1, b: 2 } as const); + + const a = result.$a; + expect(a).toEqual(1); + }); + + it('should throw a type error when the resulting object is not assigned to a variable with a $ prefix', () => { + // @ts-expect-error: here should be $ prefix for the key + const result: { + a: 1; + $b: 2; + } = makePropsTransient({ a: 1, b: 2 } as const); + expect(result).toEqual({ + $a: 1, + $b: 2, + }); + }); + it('should throw a type error when trying to get the key that is not specified in the argument object', () => { + // @ts-expect-error: c is not a key of the object + const result: { + $c: 1; + } = makePropsTransient({ a: 1, b: 2 } as const); + expect(result).toEqual({ + $a: 1, + $b: 2, + }); + }); + + it('should throw a type error when trying to access a key without a $ prefix', () => { + const result: { + $a: 1; + $b: 2; + } = makePropsTransient({ a: 1, b: 2 } as const); + + // @ts-expect-error: here should be $ prefix for the key + const { a } = result; + expect(a).toEqual(undefined); + }); + }); +}); diff --git a/packages/components/src/utils/frameProps.tsx b/packages/components/src/utils/frameProps.tsx index d00af81425e..398b835f022 100644 --- a/packages/components/src/utils/frameProps.tsx +++ b/packages/components/src/utils/frameProps.tsx @@ -56,7 +56,7 @@ type Position = { left?: string | number; }; -const cursors = ['pointer', 'help', 'default', 'not-allowed'] as const; +const cursors = ['pointer', 'help', 'default', 'not-allowed', 'inherit'] as const; type Cursor = (typeof cursors)[number]; export type FrameProps = { @@ -83,21 +83,21 @@ type TransientFrameProps = TransientProps; const getValueWithUnit = (value: string | number) => typeof value === 'number' ? `${value}px` : value; -export const pickAndPrepareFrameProps = ( - props: Record, - allowedFrameProps: Array, - convertToTransient = true, +export const pickAndPrepareFrameProps = < + TProps extends { [K in FramePropsKeys]?: FrameProps[K] }, + KFP extends FramePropsKeys[], +>( + props: TProps, + allowedFrameProps: KFP, ) => { - const selectedProps = allowedFrameProps.reduce( + const selectedProps = allowedFrameProps.reduce<{ + [value in KFP[number]]: TProps[value]; + }>( (acc, item) => ({ ...acc, [item]: props[item] }), - {}, + {} as { [value in KFP[number]]: TProps[value] }, ); - if (convertToTransient) { - return makePropsTransient(selectedProps); - } - - return selectedProps; + return makePropsTransient(selectedProps); }; export const withFrameProps = ({ diff --git a/packages/components/src/utils/transientProps.test.ts b/packages/components/src/utils/transientProps.test.ts new file mode 100644 index 00000000000..a814ccd3c48 --- /dev/null +++ b/packages/components/src/utils/transientProps.test.ts @@ -0,0 +1,28 @@ +import { makePropsTransient } from './transientProps'; + +describe('transientProps', () => { + describe('type tests', () => { + it('should return the same keys just with the $ prefix as type', () => { + const result: { + $a: 1; + $b: 2; + } = makePropsTransient({ a: 1, b: 2 } as const); + expect(result).toEqual({ + $a: 1, + $b: 2, + }); + }); + + it("should should throw an a type error when there isn't a property with a $ prefix", () => { + // @ts-expect-error: here should be $ prefixes for the keys + const result: { + a: 1; + b: 2; + } = makePropsTransient({ a: 1, b: 2 } as const); + expect(result).toEqual({ + $a: 1, + $b: 2, + }); + }); + }); +}); diff --git a/packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceSelector.tsx b/packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceSelector.tsx index 0857aade7ed..07425e5e1ea 100644 --- a/packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceSelector.tsx +++ b/packages/suite/src/components/suite/layouts/SuiteLayout/DeviceSelector/DeviceSelector.tsx @@ -77,7 +77,7 @@ export const DeviceSelector = () => { const dispatch = useDispatch(); const { getDiscoveryStatus } = useDiscovery(); const discoveryStatus = getDiscoveryStatus(); - const discoveryInProgress = discoveryStatus && discoveryStatus.status === 'loading'; + const discoveryInProgress = Boolean(discoveryStatus && discoveryStatus.status === 'loading'); const [localCount, setLocalCount] = useState(null); const [isAnimationTriggered, setIsAnimationTriggered] = useState(false); @@ -134,13 +134,11 @@ export const DeviceSelector = () => { > - ) : undefined - } + cursor={discoveryInProgress ? 'not-allowed' : undefined} + content={} > { id="TR_YOUR_FIRMWARE_VERSION" values={{ version: ( - + {revision ? ( diff --git a/yarn.lock b/yarn.lock index fbb31827581..cdf9f116354 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11580,6 +11580,15 @@ __metadata: languageName: node linkType: hard +"@testing-library/user-event@npm:^14.6.1": + version: 14.6.1 + resolution: "@testing-library/user-event@npm:14.6.1" + peerDependencies: + "@testing-library/dom": ">=7.21.4" + checksum: 10/34b74fff56a0447731a94b40d4cf246deb8dbc1c1e3aec93acd1c3377a760bb062e979f1572bb34ec164ad28ee2a391744b42d0d6d6cc16c4ce527e5e09610e1 + languageName: node + linkType: hard + "@theguild/remark-mermaid@npm:^0.0.5": version: 0.0.5 resolution: "@theguild/remark-mermaid@npm:0.0.5" @@ -11758,6 +11767,7 @@ __metadata: "@suite-common/validators": "workspace:*" "@testing-library/jest-dom": "npm:^6.6.3" "@testing-library/react": "npm:14.2.1" + "@testing-library/user-event": "npm:^14.6.1" "@trezor/asset-utils": "workspace:*" "@trezor/connect": "workspace:*" "@trezor/dom-utils": "workspace:*"