Skip to content
Open
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
8 changes: 5 additions & 3 deletions react/features/base/toolbox/components/ToolboxItem.web.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export default class ToolboxItem extends AbstractToolboxItem<IProps> {
text = { this.label } />
);
}
let children = (
const children = (
<Fragment>
{ this._renderIcon() }
{ showLabel && <span>
Expand All @@ -125,13 +125,15 @@ export default class ToolboxItem extends AbstractToolboxItem<IProps> {
);

if (useTooltip) {
children = (
const tooltip = (
<Tooltip
content = { this.tooltip ?? '' }
position = { tooltipPosition }>
{ children }
{ React.createElement(elementType, props, children) }
</Tooltip>
);

return tooltip;
}

return React.createElement(elementType, props, children);
Expand Down
65 changes: 57 additions & 8 deletions react/features/base/tooltip/components/Tooltip.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { ReactElement, useCallback, useEffect, useRef, useState } from 'react';
import React, { ReactElement, cloneElement, useCallback, useEffect, useRef, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { keyframes } from 'tss-react';
import { makeStyles } from 'tss-react/mui';
Expand Down Expand Up @@ -58,12 +58,14 @@ const Tooltip = ({ containerClassName, content, children, position = 'top' }: IP
const dispatch = useDispatch();
const [ visible, setVisible ] = useState(false);
const [ isUnmounting, setIsUnmounting ] = useState(false);
const [ wasOpenedWithKeyboard, setWasOpenedWithKeyboard ] = useState(false);
const overflowDrawer = useSelector((state: IReduxState) => state['features/toolbox'].overflowDrawer);
const { classes, cx } = useStyles();
const timeoutID = useRef({
open: 0,
close: 0
});
const tooltipId = useRef(`tooltip-${Math.random().toString(36).substring(2, 11)}`);
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

Using Math.random() for generating IDs can potentially cause collisions in applications with many tooltip instances. Consider using a more robust ID generation method like crypto.randomUUID() or a counter-based approach to ensure uniqueness.

Copilot uses AI. Check for mistakes.
const {
content: storeContent,
previousContent,
Expand All @@ -73,7 +75,10 @@ const Tooltip = ({ containerClassName, content, children, position = 'top' }: IP
const contentComponent = (
<div
className = { cx(classes.container, previousContent === '' && 'mounting-animation',
isUnmounting && 'unmounting') }>
isUnmounting && 'unmounting') }
id = { tooltipId.current }
role = 'tooltip'
tabIndex = { wasOpenedWithKeyboard ? 0 : -1 }>
{content}
</div>
);
Expand All @@ -89,31 +94,37 @@ const Tooltip = ({ containerClassName, content, children, position = 'top' }: IP
setIsUnmounting(false);
};

const onPopoverOpen = useCallback(() => {
const onPopoverOpen = useCallback((keyboardTriggered = false) => {
if (isUnmounting) {
return;
}

setWasOpenedWithKeyboard(keyboardTriggered);
clearTimeout(timeoutID.current.close);
timeoutID.current.close = 0;
if (!visible) {
if (isVisible) {
openPopover();
} else {
const delay = keyboardTriggered ? 0 : TOOLTIP_DELAY;

timeoutID.current.open = window.setTimeout(() => {
openPopover();
}, TOOLTIP_DELAY);
}, delay);
}
}
}, [ visible, isVisible, isUnmounting ]);

const onPopoverClose = useCallback(() => {
const onPopoverClose = useCallback((immediate = false) => {
clearTimeout(timeoutID.current.open);
if (visible) {
const delay = immediate ? 0 : TOOLTIP_DELAY;

timeoutID.current.close = window.setTimeout(() => {
setIsUnmounting(true);
}, TOOLTIP_DELAY);
}, delay);
}
setWasOpenedWithKeyboard(false);
}, [ visible ]);

useEffect(() => {
Expand All @@ -132,13 +143,50 @@ const Tooltip = ({ containerClassName, content, children, position = 'top' }: IP
clearTimeout(timeoutID.current.close);
timeoutID.current.close = 0;
}
}, [ storeContent ]);
}, [ storeContent, content ]);

const handleFocus = useCallback(() => {
onPopoverOpen(true);
}, [ onPopoverOpen ]);

const handleBlur = useCallback(() => {
onPopoverClose(true);
}, [ onPopoverClose ]);

const handleKeyDown = useCallback((event: React.KeyboardEvent) => {
if (event.key === 'Escape' && visible) {
event.preventDefault();
onPopoverClose(true);
}
}, [ visible, onPopoverClose ]);

if (isMobileBrowser() || overflowDrawer) {
return children;
}

const enhancedChildren = cloneElement(children, {
'aria-describedby': visible ? tooltipId.current : undefined,
tabIndex: children.props.tabIndex !== undefined ? children.props.tabIndex : 0,
Comment on lines +167 to +169
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

Setting tabIndex to 0 for all children may interfere with the natural tab order for elements that shouldn't be focusable (like divs or spans). Consider checking if the child element is naturally focusable before setting tabIndex.

Suggested change
const enhancedChildren = cloneElement(children, {
'aria-describedby': visible ? tooltipId.current : undefined,
tabIndex: children.props.tabIndex !== undefined ? children.props.tabIndex : 0,
// Helper to check if the child is natively focusable
function isNaturallyFocusable(element: ReactElement): boolean {
const focusableTags = [
'a',
'button',
'input',
'select',
'textarea'
];
// If it's a string type (native element), check tag name
if (typeof element.type === 'string') {
if (focusableTags.includes(element.type)) {
return true;
}
// <a> must have href to be focusable
if (element.type === 'a' && element.props.href) {
return true;
}
return false;
}
// For custom components, only set tabIndex if already present
return element.props.tabIndex !== undefined;
}
const enhancedChildren = cloneElement(children, {
'aria-describedby': visible ? tooltipId.current : undefined,
...(isNaturallyFocusable(children) || children.props.tabIndex !== undefined
? { tabIndex: children.props.tabIndex !== undefined ? children.props.tabIndex : 0 }
: {}),

Copilot uses AI. Check for mistakes.
onFocus: (event: React.FocusEvent) => {
handleFocus();
if (children.props.onFocus) {
children.props.onFocus(event);
}
},
onBlur: (event: React.FocusEvent) => {
handleBlur();
if (children.props.onBlur) {
children.props.onBlur(event);
}
},
onKeyDown: (event: React.KeyboardEvent) => {
handleKeyDown(event);
if (children.props.onKeyDown) {
children.props.onKeyDown(event);
}
}
});

return (
<Popover
allowClick = { true }
Expand All @@ -148,8 +196,9 @@ const Tooltip = ({ containerClassName, content, children, position = 'top' }: IP
onPopoverClose = { onPopoverClose }
onPopoverOpen = { onPopoverOpen }
position = { position }
role = 'tooltip'
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

The role='tooltip' attribute is being set on the Popover component, but tooltips already have role='tooltip' set on the content div (line 80). This creates duplicate role attributes which may confuse screen readers.

Suggested change
role = 'tooltip'

Copilot uses AI. Check for mistakes.
visible = { visible }>
{children}
{enhancedChildren}
</Popover>
);
};
Expand Down
9 changes: 6 additions & 3 deletions react/features/file-sharing/components/web/FileSharing.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { IReduxState } from '../../../app/types';
import Avatar from '../../../base/avatar/components/Avatar';
import Icon from '../../../base/icons/components/Icon';
import { IconCloudUpload, IconDownload, IconTrash } from '../../../base/icons/svg';
import Tooltip from '../../../base/tooltip/components/Tooltip';
import BaseTheme from '../../../base/ui/components/BaseTheme.web';
import Button from '../../../base/ui/components/web/Button';
import { BUTTON_TYPES } from '../../../base/ui/constants.web';
Expand Down Expand Up @@ -367,9 +368,11 @@ const FileSharing = () => {
src = { getFileIcon(file.fileType) } />
</div>
<div className = { classes.fileItemDetails }>
<div className = { classes.fileName }>
{ file.fileName }
</div>
<Tooltip content = { file.fileName }>
<div className = { classes.fileName }>
{ file.fileName }
</div>
</Tooltip>
<div className = { classes.fileSize }>
{ formatFileSize(file.fileSize) }
</div>
Expand Down