-
Notifications
You must be signed in to change notification settings - Fork 902
ATLAS-5373: Atlas React UI: Extremely long entity names break layout in Latest Entities Created widget #728
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
base: master
Are you sure you want to change the base?
Changes from all commits
6181368
ba701df
f1e77eb
0b2f658
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,11 +21,12 @@ | |
| */ | ||
|
|
||
| import React from 'react' | ||
| import { render, screen, fireEvent } from '@testing-library/react' | ||
| import { render, screen, fireEvent, act } from '@testing-library/react' | ||
| import userEvent from '@testing-library/user-event' | ||
| import { | ||
| CustomButton, | ||
| LightTooltip, | ||
| OverflowTooltip, | ||
| LinkTab, | ||
| Accordion, | ||
| AccordionSummary, | ||
|
|
@@ -69,6 +70,100 @@ describe('muiComponents', () => { | |
| expect(screen.getByText('Tooltip Child')).toBeTruthy() | ||
| }) | ||
|
|
||
| describe('OverflowTooltip', () => { | ||
| let triggerResize: any | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. triggerResize: any — prefer typed mock |
||
| const originalResizeObserver = global.ResizeObserver | ||
|
|
||
| beforeAll(() => { | ||
| global.ResizeObserver = class { | ||
| constructor(callback: any) { | ||
| triggerResize = callback | ||
| } | ||
| observe = jest.fn() | ||
| unobserve = jest.fn() | ||
| disconnect = jest.fn() | ||
| } as any | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| global.ResizeObserver = originalResizeObserver | ||
| }) | ||
|
|
||
| it('renders OverflowTooltip children', () => { | ||
| render( | ||
| <OverflowTooltip title="overflow tip"> | ||
| <span>Overflow Child</span> | ||
| </OverflowTooltip> | ||
| ) | ||
| expect(screen.getByText('Overflow Child')).toBeTruthy() | ||
| }) | ||
|
|
||
| it('disables tooltip when not overflowed', async () => { | ||
| render( | ||
| <OverflowTooltip title="overflow tip"> | ||
| <span data-testid="short-text">Short</span> | ||
| </OverflowTooltip> | ||
| ) | ||
| const span = screen.getByTestId('short-text').parentElement! | ||
|
|
||
| // Mock no overflow | ||
| Object.defineProperty(span, 'scrollWidth', { configurable: true, value: 100 }) | ||
| Object.defineProperty(span, 'clientWidth', { configurable: true, value: 100 }) | ||
|
|
||
| act(() => { | ||
| if (triggerResize) triggerResize() | ||
| }) | ||
|
|
||
| fireEvent.mouseOver(span) | ||
|
|
||
| // Tooltip should not be in the document | ||
| expect(screen.queryByText('overflow tip')).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('enables tooltip on resize if overflow occurs', async () => { | ||
| render( | ||
| <OverflowTooltip title="overflow tip"> | ||
| <span data-testid="resize-text">Will be long</span> | ||
| </OverflowTooltip> | ||
| ) | ||
| const span = screen.getByTestId('resize-text').parentElement! | ||
|
|
||
| // Mock overflow condition | ||
| Object.defineProperty(span, 'scrollWidth', { configurable: true, value: 200 }) | ||
| Object.defineProperty(span, 'clientWidth', { configurable: true, value: 100 }) | ||
|
|
||
| // Trigger resize observer callback | ||
| act(() => { | ||
| if (triggerResize) triggerResize() | ||
| }) | ||
|
|
||
| fireEvent.mouseOver(span) | ||
|
|
||
| // Tooltip should appear | ||
| expect(await screen.findByText('overflow tip')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('enables tooltip for subpixel overflow where clientWidth matches scrollWidth', async () => { | ||
| render( | ||
| <OverflowTooltip title="subpixel tip"> | ||
| <span data-testid="subpixel-text">Subpixel</span> | ||
| </OverflowTooltip> | ||
| ) | ||
| const span = screen.getByTestId('subpixel-text').parentElement! | ||
|
|
||
| // Mock subpixel overflow condition (scrollWidth matches clientWidth, but rect is smaller) | ||
| Object.defineProperty(span, 'scrollWidth', { configurable: true, value: 100 }) | ||
| Object.defineProperty(span, 'clientWidth', { configurable: true, value: 100 }) | ||
| span.getBoundingClientRect = jest.fn(() => ({ width: 99.5 } as DOMRect)) | ||
|
|
||
| // Trigger hover to fire the onMouseEnter checkOverflow logic | ||
| fireEvent.mouseEnter(span) | ||
| fireEvent.mouseOver(span) | ||
|
|
||
| expect(await screen.findByText('subpixel tip')).toBeInTheDocument() | ||
| }) | ||
| }) | ||
|
|
||
| it('prevents default navigation in LinkTab', async () => { | ||
| const preventDefault = jest.fn() | ||
| render( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,9 +22,10 @@ import Switch from "@mui/material/Switch"; | |
| import Divider from "@mui/material/Divider"; | ||
| import IconButton from "@mui/material/IconButton"; | ||
| import ListItemIcon from "@mui/material/ListItemIcon"; | ||
| import React from "react"; | ||
| import Menu from "@mui/material/Menu"; | ||
| import MenuItem from "@mui/material/MenuItem"; | ||
| import Button from "@mui/material/Button"; | ||
| import Button, { ButtonProps } from "@mui/material/Button"; | ||
| import DialogTitle from "@mui/material/DialogTitle"; | ||
| import DialogContent from "@mui/material/DialogContent"; | ||
| import DialogActions from "@mui/material/DialogActions"; | ||
|
|
@@ -51,6 +52,8 @@ import MuiAccordionSummary, { | |
| AccordionSummaryProps | ||
| } from "@mui/material/AccordionSummary"; | ||
| import MuiAccordionDetails from "@mui/material/AccordionDetails"; | ||
| import { TooltipProps } from "@mui/material/Tooltip"; | ||
| import { SxProps, Theme } from "@mui/material/styles"; | ||
|
|
||
| const LightTooltip = styled(({ className, ...props }: any) => ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LightTooltip still typed as any (pre-existing pattern, but PR claims type-safety refactor) |
||
| <Tooltip | ||
|
|
@@ -68,58 +71,94 @@ const LightTooltip = styled(({ className, ...props }: any) => ( | |
| } | ||
| })); | ||
|
|
||
| interface ButtonProps { | ||
| children?: any; | ||
| variant?: string; | ||
| color: string; | ||
| onClick: any; | ||
| sx?: any; | ||
| size?: string; | ||
| endIcon?: any; | ||
| startIcon?: any; | ||
| className?: string; | ||
| disabled?: boolean; | ||
|
|
||
| interface OverflowTooltipProps extends Omit<TooltipProps, "children"> { | ||
| children: React.ReactElement; | ||
| wrapperSx?: SxProps<Theme>; | ||
| wrapperClassName?: string; | ||
| } | ||
|
|
||
| const OverflowTooltip = ({ title, children, wrapperSx, wrapperClassName, ...props }: OverflowTooltipProps) => { | ||
| const textElementRef = React.useRef<HTMLElement>(null); | ||
| const [isOverflowed, setIsOverflowed] = React.useState(false); | ||
|
|
||
| const checkOverflow = React.useCallback(() => { | ||
| if (textElementRef.current) { | ||
| const el = textElementRef.current; | ||
| setIsOverflowed( | ||
| el.scrollWidth > el.clientWidth || | ||
| el.scrollWidth > el.getBoundingClientRect().width | ||
| ); | ||
| } | ||
| }, []); | ||
|
|
||
| React.useEffect(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One ResizeObserver per list row (up to 7) — acceptable for this widget, but worth noting if reused widely |
||
| checkOverflow(); | ||
| const element = textElementRef.current; | ||
| if (element) { | ||
| const resizeObserver = new ResizeObserver(() => checkOverflow()); | ||
| resizeObserver.observe(element); | ||
| return () => resizeObserver.disconnect(); | ||
| } | ||
| }, [title, checkOverflow]); | ||
|
|
||
| const child = ( | ||
| <Box | ||
| component="span" | ||
| ref={textElementRef} | ||
| className={wrapperClassName} | ||
| sx={{ | ||
| display: "inline-flex", | ||
| minWidth: 0, | ||
| width: "100%", | ||
| overflow: "hidden", | ||
| textOverflow: "ellipsis", | ||
| whiteSpace: "nowrap", | ||
| ...wrapperSx | ||
| }} | ||
| onMouseEnter={checkOverflow} | ||
| > | ||
| {children} | ||
| </Box> | ||
| ); | ||
|
|
||
| return ( | ||
| <LightTooltip | ||
| title={title} | ||
| disableHoverListener={!isOverflowed} | ||
| disableFocusListener={!isOverflowed} | ||
| disableTouchListener={!isOverflowed} | ||
| {...props} | ||
| > | ||
| {child} | ||
| </LightTooltip> | ||
| ); | ||
| }; | ||
|
|
||
| const ButtonWrapper = styled(Box)({ | ||
| display: "inline-flex" | ||
| }); | ||
|
|
||
| const StyledButton = styled(Button)(({ variant }) => ({ | ||
| fontWeight: "600", | ||
| letterSpacing: "0", | ||
| fontSize: "0.875rem", | ||
| cursor: "pointer", | ||
| minWidth: "unset", | ||
| ...(variant === "outlined" && { border: "1px solid #dddddd" }) | ||
| })); | ||
|
|
||
| const CustomButton = ({ | ||
| children, | ||
| variant, | ||
| color, | ||
| sx: customStyles = {}, | ||
| onClick, | ||
| size, | ||
| endIcon, | ||
| startIcon, | ||
| disabled, | ||
| sx, | ||
| ...rest | ||
| }: ButtonProps | any) => { | ||
| let defaultStyles = { | ||
| fontWeight: "600 !important", | ||
| letterSpacing: "0 !important", | ||
| fontSize: "0.875rem !important", | ||
| cursor: "pointer !important", | ||
| minWidth: "unset !important", | ||
| ...(variant == "outlined" && { border: "1px solid #dddddd !important" }) | ||
| }; | ||
|
|
||
| let mergedStyle = { ...defaultStyles, ...customStyles }; | ||
|
|
||
| }: ButtonProps) => { | ||
| return ( | ||
| <Box component="span" sx={{ display: 'inline-flex' }}> | ||
| <Button | ||
| variant={variant} | ||
| color={color} | ||
| sx={mergedStyle} | ||
| onClick={onClick} | ||
| size={size} | ||
| endIcon={endIcon} | ||
| startIcon={startIcon} | ||
| disabled={disabled} | ||
| {...rest} | ||
| > | ||
| <ButtonWrapper component="span"> | ||
| <StyledButton sx={sx} {...rest}> | ||
| {children} | ||
| </Button> | ||
| </Box> | ||
| </StyledButton> | ||
| </ButtonWrapper> | ||
| ); | ||
| }; | ||
|
|
||
|
|
@@ -202,5 +241,6 @@ export { | |
| CustomButton, | ||
| Accordion, | ||
| AccordionSummary, | ||
| AccordionDetails | ||
| AccordionDetails, | ||
| OverflowTooltip | ||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.