From e3f1d4163ae716e0d3d50d85363219b0dfdb4beb Mon Sep 17 00:00:00 2001 From: Diwaspant132 Date: Fri, 23 Oct 2026 21:35:06 +0545 Subject: [PATCH] fix(ui-elements): center tooltip alignment, add text crash guard, and fix ActionButton icon overflow Signed-off-by: Diwaspant132 --- packages/ui-elements/package.json | 3 +- .../components/ActionButton/ActionButton.js | 22 +++--- .../ActionButton/ActionButton.test.mjs | 36 +++++++++ .../components/ActionButton/getIconSize.js | 35 +++++++++ .../src/components/Tooltip/Tooltip.js | 76 +++++++++++++++++-- .../src/components/Tooltip/Tooltip.styles.js | 30 +++++--- .../Tooltip/Tooltip.styles.test.mjs | 74 ++++++++++++++++++ .../src/components/Tooltip/Tooltip.test.mjs | 55 ++++++++++++++ 8 files changed, 302 insertions(+), 29 deletions(-) create mode 100644 packages/ui-elements/src/components/ActionButton/ActionButton.test.mjs create mode 100644 packages/ui-elements/src/components/ActionButton/getIconSize.js create mode 100644 packages/ui-elements/src/components/Tooltip/Tooltip.styles.test.mjs create mode 100644 packages/ui-elements/src/components/Tooltip/Tooltip.test.mjs diff --git a/packages/ui-elements/package.json b/packages/ui-elements/package.json index e08f04fb6f..869b976f00 100644 --- a/packages/ui-elements/package.json +++ b/packages/ui-elements/package.json @@ -22,7 +22,8 @@ "lint": "eslint 'src/**/*.js'", "lint:fix": "eslint 'src/**/*.js' --fix", "storybook": "storybook dev -p 6006", - "build-storybook": "storybook build" + "build-storybook": "storybook build", + "test": "node --test src/components/**/*.test.mjs" }, "author": "", "license": "ISC", diff --git a/packages/ui-elements/src/components/ActionButton/ActionButton.js b/packages/ui-elements/src/components/ActionButton/ActionButton.js index f6dfb25f7e..64c6a8e541 100644 --- a/packages/ui-elements/src/components/ActionButton/ActionButton.js +++ b/packages/ui-elements/src/components/ActionButton/ActionButton.js @@ -2,16 +2,7 @@ import React, { forwardRef } from 'react'; import PropTypes from 'prop-types'; import { Button } from '../Button'; import { Icon } from '../Icon'; - -const getSize = ({ small, large, size }) => { - if (small) { - return '1.25rem'; - } - if (large) { - return '1.75rem'; - } - return size || '1.5rem'; -}; +import getIconSize from './getIconSize'; const ActionButton = forwardRef( ( @@ -27,7 +18,7 @@ const ActionButton = forwardRef( ) => ( ) ); @@ -41,6 +32,15 @@ ActionButton.propTypes = { PropTypes.node, ]), size: PropTypes.oneOf(['small', 'medium', 'large']), + iconSize: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.shape({ + small: PropTypes.bool, + large: PropTypes.bool, + size: PropTypes.string, + }), + ]), }; +export { getIconSize }; export default ActionButton; diff --git a/packages/ui-elements/src/components/ActionButton/ActionButton.test.mjs b/packages/ui-elements/src/components/ActionButton/ActionButton.test.mjs new file mode 100644 index 0000000000..0670b2414d --- /dev/null +++ b/packages/ui-elements/src/components/ActionButton/ActionButton.test.mjs @@ -0,0 +1,36 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import getIconSize from './getIconSize.js'; + +test('ActionButton: derives small icon size (1rem) for small action button to prevent container overflow', () => { + assert.strictEqual(getIconSize({}, 'small'), '1rem'); +}); + +test('ActionButton: derives medium icon size (1.25rem) by default', () => { + assert.strictEqual(getIconSize({}, 'medium'), '1.25rem'); + assert.strictEqual(getIconSize(), '1.25rem'); +}); + +test('ActionButton: derives large icon size (1.75rem) for large action button', () => { + assert.strictEqual(getIconSize({}, 'large'), '1.75rem'); +}); + +test('ActionButton: supports named iconSize strings ("small", "medium", "large")', () => { + assert.strictEqual(getIconSize('small', 'medium'), '1rem'); + assert.strictEqual(getIconSize('medium', 'medium'), '1.25rem'); + assert.strictEqual(getIconSize('large', 'medium'), '1.75rem'); +}); + +test('ActionButton: supports custom iconSize css length string', () => { + assert.strictEqual(getIconSize('1.4rem', 'medium'), '1.4rem'); + assert.strictEqual(getIconSize('18px', 'small'), '18px'); +}); + +test('ActionButton: supports legacy object syntax { small: true } and { large: true }', () => { + assert.strictEqual(getIconSize({ small: true }), '1rem'); + assert.strictEqual(getIconSize({ large: true }), '1.75rem'); +}); + +test('ActionButton: supports explicit size property in iconSize object { size: "1.3rem" }', () => { + assert.strictEqual(getIconSize({ size: '1.3rem' }), '1.3rem'); +}); diff --git a/packages/ui-elements/src/components/ActionButton/getIconSize.js b/packages/ui-elements/src/components/ActionButton/getIconSize.js new file mode 100644 index 0000000000..6f00a8cabb --- /dev/null +++ b/packages/ui-elements/src/components/ActionButton/getIconSize.js @@ -0,0 +1,35 @@ +const getIconSize = (iconSize, buttonSize) => { + if (typeof iconSize === 'string') { + if (iconSize === 'small') { + return '1rem'; + } + if (iconSize === 'medium') { + return '1.25rem'; + } + if (iconSize === 'large') { + return '1.75rem'; + } + return iconSize; + } + if (iconSize && typeof iconSize === 'object') { + if (iconSize.size) { + return iconSize.size; + } + if (iconSize.small) { + return '1rem'; + } + if (iconSize.large) { + return '1.75rem'; + } + } + if (buttonSize === 'small') { + return '1rem'; + } + if (buttonSize === 'large') { + return '1.75rem'; + } + return '1.25rem'; +}; + +export default getIconSize; + diff --git a/packages/ui-elements/src/components/Tooltip/Tooltip.js b/packages/ui-elements/src/components/Tooltip/Tooltip.js index cc3eab6801..7b1436b17d 100644 --- a/packages/ui-elements/src/components/Tooltip/Tooltip.js +++ b/packages/ui-elements/src/components/Tooltip/Tooltip.js @@ -1,11 +1,20 @@ -import React, { useState } from 'react'; +import React, { useState, useRef, useEffect } from 'react'; import { css } from '@emotion/react'; +import PropTypes from 'prop-types'; import { Box } from '../Box'; import getToolTipStyles from './Tooltip.styles'; import { useTheme } from '../../hooks'; -const Tooltip = ({ children, text, position }) => { +const Tooltip = ({ + children, + text, + position = 'top', + className = '', + style = {}, + ...props +}) => { const [isTooltipVisible, setTooltipVisible] = useState(false); + const touchTimerRef = useRef(null); const { theme } = useTheme(); const styles = getToolTipStyles(theme, position); @@ -17,35 +26,78 @@ const Tooltip = ({ children, text, position }) => { setTooltipVisible(false); }; - let touchTimer; + const handleFocus = () => { + setTooltipVisible(true); + }; + + const handleBlur = () => { + setTooltipVisible(false); + }; + + const handleKeyDown = (event) => { + if (event.key === 'Escape') { + setTooltipVisible(false); + } + }; const handleTouchStart = () => { - touchTimer = setTimeout(() => { + touchTimerRef.current = setTimeout(() => { setTooltipVisible(true); }, 500); }; const handleTouchEnd = () => { - clearTimeout(touchTimer); + if (touchTimerRef.current) { + clearTimeout(touchTimerRef.current); + touchTimerRef.current = null; + } setTooltipVisible(false); }; + useEffect( + () => () => { + if (touchTimerRef.current) { + clearTimeout(touchTimerRef.current); + } + }, + [] + ); + + const hasText = Boolean( + text && (typeof text === 'string' ? text.trim().length > 0 : true) + ); + const formattedText = + typeof text === 'string' && text.length > 0 + ? text.charAt(0).toUpperCase() + text.slice(1) + : text; + return ( {children} - {isTooltipVisible && ( - - {text.charAt(0).toUpperCase() + text.slice(1)} + {hasText && isTooltipVisible && ( + + {formattedText} )} @@ -53,4 +105,12 @@ const Tooltip = ({ children, text, position }) => { ); }; +Tooltip.propTypes = { + children: PropTypes.node, + text: PropTypes.node, + position: PropTypes.oneOf(['top', 'bottom', 'down']), + className: PropTypes.string, + style: PropTypes.object, +}; + export default Tooltip; diff --git a/packages/ui-elements/src/components/Tooltip/Tooltip.styles.js b/packages/ui-elements/src/components/Tooltip/Tooltip.styles.js index 824eec5d49..7fec027b38 100644 --- a/packages/ui-elements/src/components/Tooltip/Tooltip.styles.js +++ b/packages/ui-elements/src/components/Tooltip/Tooltip.styles.js @@ -1,10 +1,12 @@ import { css } from '@emotion/react'; -const getTooltipStyles = (theme, position) => { +const getTooltipStyles = (theme, position = 'top') => { + const isBottom = position === 'bottom' || position === 'down'; + const styles = { tooltip: css` position: absolute; - left: 64%; + left: 50%; transform: translateX(-50%); background-color: ${theme.invertedColors.secondary}; color: ${theme.invertedColors.secondaryForeground}; @@ -16,22 +18,32 @@ const getTooltipStyles = (theme, position) => { font-weight: 500; white-space: nowrap; font-family: sans-serif; - top: ${position === 'top' ? 'calc(-100% - 20px)' : 'calc(100% + 10px)'}; + pointer-events: none; + ${isBottom + ? css` + top: calc(100% + 8px); + ` + : css` + bottom: calc(100% + 8px); + `} `, tooltipArrow: css` content: ''; position: absolute; left: 50%; - margin-left: -4px; border-width: 6px; border-style: solid; border-color: ${theme.invertedColors.secondary} transparent transparent transparent; - top: ${position === 'top' ? '100%' : 'auto'}; - bottom: ${position === 'bottom' ? '100%' : 'auto'}; - transform: ${position === 'bottom' - ? 'translateX(-50%) rotate(180deg)' - : 'translateX(-50%)'}; + ${isBottom + ? css` + bottom: 100%; + transform: translateX(-50%) rotate(180deg); + ` + : css` + top: 100%; + transform: translateX(-50%); + `} `, }; diff --git a/packages/ui-elements/src/components/Tooltip/Tooltip.styles.test.mjs b/packages/ui-elements/src/components/Tooltip/Tooltip.styles.test.mjs new file mode 100644 index 0000000000..aa4722f332 --- /dev/null +++ b/packages/ui-elements/src/components/Tooltip/Tooltip.styles.test.mjs @@ -0,0 +1,74 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import getTooltipStyles from './Tooltip.styles.js'; + +const mockTheme = { + radius: '4px', + invertedColors: { + secondary: '#1f2328', + secondaryForeground: '#ffffff', + }, + zIndex: { + tooltip: 1400, + }, +}; + +test('Tooltip.styles: centers tooltip horizontally with left: 50% and translateX(-50%)', () => { + const styles = getTooltipStyles(mockTheme, 'top'); + const tooltipCss = styles.tooltip.styles; + + assert.match(tooltipCss, /left:\s*50%/); + assert.match(tooltipCss, /transform:\s*translateX\(-50%\)/); + assert.doesNotMatch( + tooltipCss, + /left:\s*64%/, + 'Should not use hardcoded 64% offset' + ); +}); + +test('Tooltip.styles: positions top tooltip above anchor element', () => { + const styles = getTooltipStyles(mockTheme, 'top'); + const tooltipCss = styles.tooltip.styles; + const arrowCss = styles.tooltipArrow.styles; + + assert.match(tooltipCss, /bottom:\s*calc\(100%\s*\+\s*8px\)/); + assert.match(arrowCss, /top:\s*100%/); + assert.match(arrowCss, /transform:\s*translateX\(-50%\)/); +}); + +test('Tooltip.styles: positions bottom tooltip below anchor element with inverted arrow', () => { + const styles = getTooltipStyles(mockTheme, 'bottom'); + const tooltipCss = styles.tooltip.styles; + const arrowCss = styles.tooltipArrow.styles; + + assert.match(tooltipCss, /top:\s*calc\(100%\s*\+\s*8px\)/); + assert.match(arrowCss, /bottom:\s*100%/); + assert.match(arrowCss, /rotate\(180deg\)/); +}); + +test('Tooltip.styles: normalizes position "down" to behave identically to "bottom"', () => { + const downStyles = getTooltipStyles(mockTheme, 'down'); + const bottomStyles = getTooltipStyles(mockTheme, 'bottom'); + + assert.strictEqual(downStyles.tooltip.styles, bottomStyles.tooltip.styles); + assert.strictEqual( + downStyles.tooltipArrow.styles, + bottomStyles.tooltipArrow.styles + ); +}); + +test('Tooltip.styles: defaults to "top" position when position argument is omitted', () => { + const defaultStyles = getTooltipStyles(mockTheme); + const topStyles = getTooltipStyles(mockTheme, 'top'); + + assert.strictEqual(defaultStyles.tooltip.styles, topStyles.tooltip.styles); + assert.strictEqual( + defaultStyles.tooltipArrow.styles, + topStyles.tooltipArrow.styles + ); +}); + +test('Tooltip.styles: includes pointer-events: none to avoid hover jitter and blocking clicks', () => { + const styles = getTooltipStyles(mockTheme, 'top'); + assert.match(styles.tooltip.styles, /pointer-events:\s*none/); +}); diff --git a/packages/ui-elements/src/components/Tooltip/Tooltip.test.mjs b/packages/ui-elements/src/components/Tooltip/Tooltip.test.mjs new file mode 100644 index 0000000000..8ec1266610 --- /dev/null +++ b/packages/ui-elements/src/components/Tooltip/Tooltip.test.mjs @@ -0,0 +1,55 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +// Helper mirroring Tooltip's text formatting logic +const formatTooltipText = (text) => { + const hasText = Boolean( + text && (typeof text === 'string' ? text.trim().length > 0 : true) + ); + const formattedText = + typeof text === 'string' && text.length > 0 + ? text.charAt(0).toUpperCase() + text.slice(1) + : text; + + return { hasText, formattedText }; +}; + +test('Tooltip formatTooltipText: safely handles undefined without throwing charAt TypeError', () => { + assert.doesNotThrow(() => { + const { hasText, formattedText } = formatTooltipText(undefined); + assert.strictEqual(hasText, false); + assert.strictEqual(formattedText, undefined); + }); +}); + +test('Tooltip formatTooltipText: safely handles null without throwing TypeError', () => { + assert.doesNotThrow(() => { + const { hasText, formattedText } = formatTooltipText(null); + assert.strictEqual(hasText, false); + assert.strictEqual(formattedText, null); + }); +}); + +test('Tooltip formatTooltipText: safely marks empty string as hasText = false', () => { + const { hasText, formattedText } = formatTooltipText(''); + assert.strictEqual(hasText, false); + assert.strictEqual(formattedText, ''); +}); + +test('Tooltip formatTooltipText: marks whitespace-only string as hasText = false', () => { + const { hasText } = formatTooltipText(' '); + assert.strictEqual(hasText, false); +}); + +test('Tooltip formatTooltipText: capitalizes first letter of valid tooltip string', () => { + const { hasText, formattedText } = formatTooltipText('pinned'); + assert.strictEqual(hasText, true); + assert.strictEqual(formattedText, 'Pinned'); +}); + +test('Tooltip formatTooltipText: preserves already capitalized string', () => { + const { hasText, formattedText } = formatTooltipText('Followers (3)'); + assert.strictEqual(hasText, true); + assert.strictEqual(formattedText, 'Followers (3)'); +}); +