diff --git a/packages/ui-breadcrumb/src/Breadcrumb/__tests__/Breadcrumb.test.tsx b/packages/ui-breadcrumb/src/Breadcrumb/__tests__/Breadcrumb.test.tsx index dc1bae6526..3082b49ddf 100644 --- a/packages/ui-breadcrumb/src/Breadcrumb/__tests__/Breadcrumb.test.tsx +++ b/packages/ui-breadcrumb/src/Breadcrumb/__tests__/Breadcrumb.test.tsx @@ -65,6 +65,32 @@ describe('', () => { expect(axeCheck).toBe(true) }) + it('should stay on a single line when the crumbs do not fit', async () => { + const crumbs = ( + + English literature 204 + Second term modules + Current lesson + + ) + const { container } = await render( +
+
+ {crumbs} +
+
+ {crumbs} +
+
+ ) + const [wideList, narrowList] = [...container.querySelectorAll('ol')].map( + (list) => list.getBoundingClientRect() + ) + + expect(narrowList.width).toBeLessThan(wideList.width) + expect(narrowList.height).toBe(wideList.height) + }) + it('should render the label as an aria-label attribute', async () => { await render( diff --git a/packages/ui-breadcrumb/src/Breadcrumb/__tests__/BreadcrumbLink.test.tsx b/packages/ui-breadcrumb/src/Breadcrumb/__tests__/BreadcrumbLink.test.tsx index 1a70cb8bd9..c42ad4cfb1 100644 --- a/packages/ui-breadcrumb/src/Breadcrumb/__tests__/BreadcrumbLink.test.tsx +++ b/packages/ui-breadcrumb/src/Breadcrumb/__tests__/BreadcrumbLink.test.tsx @@ -124,6 +124,70 @@ describe('', () => { expect(span).toMatchTextContent(TEST_TEXT_01) }) + describe('truncation', () => { + const LONG_TEXT = 'A very long breadcrumb text that does not fit' + + it('should truncate long text with CSS and keep the full text in the DOM', async () => { + const { container } = await render( +
+ {LONG_TEXT} +
+ ) + const link = page.getByRole('link').element() + const text = container.querySelector('a > span')! + + expect(link).toMatchTextContent(LONG_TEXT) + expect(text).toHaveStyle('white-space: nowrap') + expect(text).toHaveStyle('text-overflow: ellipsis') + expect(text.scrollWidth).toBeGreaterThan(text.clientWidth) + }) + + it('should truncate long text next to an icon', async () => { + const { container } = await render( +
+ } + > + {LONG_TEXT} + +
+ ) + const link = page.getByRole('link').element() + const text = container.querySelector('a > span:last-child')! + const icon = page.getByTestId('icon').element() + + expect(text).toMatchTextContent(LONG_TEXT) + expect(text.scrollWidth).toBeGreaterThan(text.clientWidth) + expect(link.getBoundingClientRect().height).toBeLessThan( + icon.getBoundingClientRect().height * 2 + ) + }) + + it('should show the full text in a tooltip only when truncated', async () => { + await render( +
+
+ {LONG_TEXT} +
+ {TEST_TEXT_01} +
+ ) + const [truncatedLink, shortLink] = page.getByRole('link').all() + + await userEvent.hover(truncatedLink) + await expect + .element(page.getByRole('tooltip')) + .toMatchTextContent(LONG_TEXT) + + await userEvent.unhover(truncatedLink) + await expect.element(page.getByRole('tooltip')).not.toBeInTheDocument() + + await userEvent.hover(shortLink) + await expect.element(page.getByRole('tooltip')).not.toBeInTheDocument() + }) + }) + it('should meet a11y standards as a link', async () => { const { container } = await render( {TEST_TEXT_01} diff --git a/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/index.tsx b/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/index.tsx index b7aceeea23..0b6046485a 100644 --- a/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/index.tsx +++ b/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/index.tsx @@ -24,10 +24,12 @@ import { Component } from 'react' -import { TruncateText } from '@instructure/ui-truncate-text/latest' import { Link } from '@instructure/ui-link/latest' import { omitProps } from '@instructure/ui-react-utils' import { Tooltip } from '@instructure/ui-tooltip/latest' +import { withStyleNew } from '@instructure/emotion' + +import generateStyle from './styles.js' import { allowedProps } from './props.js' import type { BreadcrumbLinkProps, BreadcrumbLinkState } from './props' @@ -39,6 +41,7 @@ id: Breadcrumb.Link --- **/ +@withStyleNew(generateStyle) class BreadcrumbLink extends Component< BreadcrumbLinkProps, BreadcrumbLinkState @@ -50,10 +53,17 @@ class BreadcrumbLink extends Component< static defaultProps = {} ref: Element | null = null + private _textRef: HTMLSpanElement | null = null + private _resizeObserver?: ResizeObserver handleRef = (el: Element | null) => { this.ref = el } + + handleTextRef = (el: HTMLSpanElement | null) => { + this._textRef = el + } + constructor(props: BreadcrumbLinkProps) { super(props) @@ -61,7 +71,32 @@ class BreadcrumbLink extends Component< isTruncated: false } } - handleTruncation(isTruncated: boolean) { + + componentDidMount() { + this.props.makeStyles?.() + + if (this._textRef && typeof ResizeObserver !== 'undefined') { + this._resizeObserver = new ResizeObserver(this.checkTruncation) + this._resizeObserver.observe(this._textRef) + } + this.checkTruncation() + } + + componentDidUpdate() { + this.props.makeStyles?.() + this.checkTruncation() + } + + componentWillUnmount() { + this._resizeObserver?.disconnect() + } + + checkTruncation = () => { + if (!this._textRef) { + return + } + const isTruncated = this._textRef.scrollWidth > this._textRef.clientWidth + if (isTruncated !== this.state.isTruncated) { this.setState({ isTruncated }) } @@ -76,7 +111,8 @@ class BreadcrumbLink extends Component< onClick, onMouseEnter, isCurrentPage, - size + size, + styles } = this.props const { isTruncated } = this.state const props = omitProps(this.props, BreadcrumbLink.allowedProps) @@ -87,7 +123,7 @@ class BreadcrumbLink extends Component< renderTip={children} preventTooltip={!isTruncated} // this wraps the achor/button tag in a span and puts the aria-describedby on that instead of the anchor/button tag - // to avoid SRs reading the text twice when there is already an aria-label + // to avoid SRs reading the text twice {...(isInteractive && { as: 'span' })} > - this.handleTruncation(isTruncated)} - > - {children} - + + {children} + ) diff --git a/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/props.ts b/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/props.ts index ff46883d19..9082f19ba6 100644 --- a/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/props.ts +++ b/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/props.ts @@ -30,6 +30,7 @@ import type { } from '@instructure/shared-types' import type { ViewOwnProps } from '@instructure/ui-view/latest' import type { LinkProps } from '@instructure/ui-link/latest' +import type { WithStyleProps, ComponentStyle } from '@instructure/emotion' type BreadcrumbLinkOwnProps = { /** @@ -81,7 +82,10 @@ type BreadcrumbLinkProps = PickPropsWithExceptions< | 'elementRef' > & BreadcrumbLinkOwnProps & + WithStyleProps & OtherHTMLAttributes + +type BreadcrumbLinkStyle = ComponentStyle<'text'> const allowedProps: AllowedPropKeys = [ 'children', 'href', @@ -97,5 +101,5 @@ type BreadcrumbLinkState = { isTruncated: boolean } -export type { BreadcrumbLinkProps, BreadcrumbLinkState } +export type { BreadcrumbLinkProps, BreadcrumbLinkState, BreadcrumbLinkStyle } export { allowedProps } diff --git a/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/styles.ts b/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/styles.ts new file mode 100644 index 0000000000..24a3054863 --- /dev/null +++ b/packages/ui-breadcrumb/src/Breadcrumb/v2/BreadcrumbLink/styles.ts @@ -0,0 +1,47 @@ +/* + * The MIT License (MIT) + * + * Copyright (c) 2015 - present Instructure, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import type { BreadcrumbLinkStyle } from './props' + +/** + * --- + * private: true + * --- + * Generates the style object from the theme and provided additional information + * @param {Object} _componentTheme The theme variable object. + * @return {Object} The final style object, which will be used in the component + */ +const generateStyle = (_componentTheme: never): BreadcrumbLinkStyle => { + return { + text: { + label: 'breadcrumbLink__text', + display: 'block', + overflow: 'hidden', + textOverflow: 'ellipsis', + whiteSpace: 'nowrap' + } + } +} + +export default generateStyle diff --git a/packages/ui-breadcrumb/src/Breadcrumb/v2/styles.ts b/packages/ui-breadcrumb/src/Breadcrumb/v2/styles.ts index 16ad4e6bec..f5b02aa8bb 100644 --- a/packages/ui-breadcrumb/src/Breadcrumb/v2/styles.ts +++ b/packages/ui-breadcrumb/src/Breadcrumb/v2/styles.ts @@ -60,18 +60,7 @@ const generateStyle = ( crumb: { label: 'breadcrumb__crumb', boxSizing: 'border-box', - display: 'flex', - alignItems: 'center', - - // prevent text clipping - '[data-cid~="TruncateText"]': { - overflow: 'visible', - - // prevent extra spacing after the '...' - '& [class*="truncateText__spacer"]': { - display: 'inline' - } - } + display: 'block' }, separator: { label: 'breadcrumb__separator', diff --git a/packages/ui-link/src/Link/v2/index.tsx b/packages/ui-link/src/Link/v2/index.tsx index fa34447bbf..d3c4c7a1e2 100644 --- a/packages/ui-link/src/Link/v2/index.tsx +++ b/packages/ui-link/src/Link/v2/index.tsx @@ -25,7 +25,6 @@ import { Children, Component } from 'react' import { View } from '@instructure/ui-view/latest' -import { hasVisibleChildren } from '@instructure/ui-a11y-utils' import { isActiveElement, findFocusable } from '@instructure/ui-dom-utils' import { getElementType, @@ -41,7 +40,7 @@ import { withStyleNew } from '@instructure/emotion' import generateStyle from './styles.js' import { allowedProps } from './props.js' -import type { LinkProps, LinkState, LinkStyleProps } from './props' +import type { LinkProps, LinkState } from './props' import type { ViewOwnProps } from '@instructure/ui-view/latest' @@ -69,31 +68,11 @@ class Link extends Component { ref: Element | null = null componentDidMount() { - this.props.makeStyles?.(this.makeStyleProps()) + this.props.makeStyles?.() } componentDidUpdate() { - this.props.makeStyles?.(this.makeStyleProps()) - } - - makeStyleProps = (): LinkStyleProps & { - variant?: LinkProps['variant'] - size?: LinkProps['size'] - } => { - const { variant, size: sizeProp } = this.props - - let size = sizeProp - - if ((variant === 'inline' || variant === 'standalone') && !sizeProp) { - size = 'medium' - } - - return { - containsTruncateText: this.containsTruncateText, - hasVisibleChildren: this.hasVisibleChildren, - variant, - size - } + this.props.makeStyles?.() } handleElementRef = (el: Element | null) => { @@ -180,10 +159,6 @@ class Link extends Component { return findFocusable(this.ref) } - get hasVisibleChildren() { - return hasVisibleChildren(this.props.children) - } - get role() { const { role, forceButtonRole, onClick } = this.props diff --git a/packages/ui-link/src/Link/v2/props.ts b/packages/ui-link/src/Link/v2/props.ts index f1378e6545..2286dbfcd0 100644 --- a/packages/ui-link/src/Link/v2/props.ts +++ b/packages/ui-link/src/Link/v2/props.ts @@ -136,11 +136,6 @@ type LinkOwnProps = { size?: 'small' | 'medium' | 'large' } -export type LinkStyleProps = { - containsTruncateText: boolean - hasVisibleChildren: boolean -} - type PropKeys = keyof LinkOwnProps type AllowedPropKeys = Readonly> diff --git a/packages/ui-link/src/Link/v2/styles.ts b/packages/ui-link/src/Link/v2/styles.ts index 20e0f758f8..2bef0f877f 100644 --- a/packages/ui-link/src/Link/v2/styles.ts +++ b/packages/ui-link/src/Link/v2/styles.ts @@ -23,7 +23,8 @@ */ import { NewComponentTypes, SharedTokens } from '@instructure/ui-themes' -import type { LinkProps, LinkStyle, LinkStyleProps } from './props' +import { hasVisibleChildren } from '@instructure/ui-a11y-utils' +import type { LinkProps, LinkStyle } from './props' import { calcFocusOutlineStyles, calcSpacingFromShorthand @@ -36,31 +37,27 @@ import { * @param {Object} componentTheme The theme variable object. * @param {Object} props the props of the component, the style is applied to * @param {Object} sharedTokens Shared token object that stores common values for the theme. - * @param {Object} state the state of the component, the style is applied to * @return {Object} The final style object, which will be used in the component */ const generateStyle = ( componentTheme: ReturnType, props: LinkProps, - sharedTokens: SharedTokens, - state: Partial< - LinkStyleProps & { - variant?: 'inline' | 'standalone' - size?: 'small' | 'medium' | 'large' - } - > = {} + sharedTokens: SharedTokens ): LinkStyle => { const { renderIcon, iconPlacement = 'start', // TODO workaround needed for react 19 where defaultprops doesn't apply for some reasong color, - margin + margin, + variant, + children } = props - // Get size and variant from state (passed from makeStyleProps) or fall back to props - const size = state.size ?? props.size - const variant = state.variant ?? props.variant - const hasVisibleChildren = state.hasVisibleChildren ?? false + const size = + !props.size && (variant === 'inline' || variant === 'standalone') + ? 'medium' + : props.size + const hasVisibleContent = hasVisibleChildren(children) const isInverseStyle = color === 'link-inverse' const inlineLinkSizeStyles = { @@ -152,7 +149,7 @@ const generateStyle = ( // If icon is present, use flex to align icon with text // Use 'flex' for standalone variant (block-level), 'inline-flex' for inline variant ...(renderIcon && - hasVisibleChildren && { + hasVisibleContent && { display: variant === 'standalone' ? 'flex' : 'inline-flex', alignItems: 'baseline' }), diff --git a/packages/ui-table/src/Table/v2/Row/index.tsx b/packages/ui-table/src/Table/v2/Row/index.tsx index 8067a7b8ec..27777c8fbf 100644 --- a/packages/ui-table/src/Table/v2/Row/index.tsx +++ b/packages/ui-table/src/Table/v2/Row/index.tsx @@ -23,9 +23,9 @@ */ import { - Component, + forwardRef, + useContext, Children, - ContextType, isValidElement, type ReactElement } from 'react' @@ -33,7 +33,7 @@ import { import { omitProps, safeCloneElement } from '@instructure/ui-react-utils' import { View } from '@instructure/ui-view/latest' -import { withStyleNew } from '@instructure/emotion' +import { useStyleNew } from '@instructure/emotion' import generateStyle from './styles.js' @@ -47,60 +47,52 @@ parent: Table id: Table.Row --- **/ -@withStyleNew(generateStyle) -class Row extends Component { - static displayName = 'Row' - static readonly componentId = 'Table.Row' - static contextType = TableContext - declare context: ContextType - static allowedProps = allowedProps +const Row = forwardRef((props, ref) => { + const { children, setHoverStateTo, themeOverride } = props + const { isStacked, hover, headers } = useContext(TableContext) - static defaultProps = { - children: null - } - - componentDidMount() { - this.props.makeStyles?.({ - isStacked: this.context.isStacked, - hover: this.context.hover - }) - } + const styles = useStyleNew({ + generateStyle, + themeOverride, + params: { isStacked, hover, setHoverStateTo }, + componentId: 'TableRow', + displayName: 'Row' + }) - componentDidUpdate() { - this.props.makeStyles?.({ - isStacked: this.context.isStacked, - hover: this.context.hover - }) + const handleElementRef = (el: HTMLElement | null) => { + if (typeof ref === 'function') { + ref(el) + } else if (ref) { + const refObject = ref as React.MutableRefObject + refObject.current = el + } } - render() { - const { children, styles } = this.props - const isStacked = this.context.isStacked - const headers = this.context.headers + return ( + + {Children.toArray(children) + .filter(Boolean) + .map((child, index) => { + if (isValidElement(child)) { + return safeCloneElement(child, { + key: (child as ReactElement).props.name, + // used by `Cell` to render its column title in `stacked` layout + header: headers && headers[index] + }) + } + return child + })} + + ) +}) - return ( - - {Children.toArray(children) - .filter(Boolean) - .map((child, index) => { - if (isValidElement(child)) { - return safeCloneElement(child, { - key: (child as ReactElement).props.name, - // used by `Cell` to render its column title in `stacked` layout - header: headers && headers[index] - }) - } - return child - })} - - ) - } -} +Row.displayName = 'Row' export default Row export { Row } diff --git a/packages/ui-table/src/Table/v2/Row/props.ts b/packages/ui-table/src/Table/v2/Row/props.ts index dae2bffd7c..b362874cee 100644 --- a/packages/ui-table/src/Table/v2/Row/props.ts +++ b/packages/ui-table/src/Table/v2/Row/props.ts @@ -24,7 +24,7 @@ import React from 'react' import type { OtherHTMLAttributes } from '@instructure/shared-types' -import type { ComponentStyle, WithStyleProps } from '@instructure/emotion' +import type { ComponentStyle, NewThemeOverrideProp } from '@instructure/emotion' import type { NewComponentTypes } from '@instructure/ui-themes' type TableRowOwnProps = { @@ -57,7 +57,7 @@ type PropKeys = keyof TableRowOwnProps type AllowedPropKeys = Readonly> type TableRowProps = TableRowOwnProps & - WithStyleProps, TableRowStyle> & + NewThemeOverrideProp> & OtherHTMLAttributes type TableRowStyle = ComponentStyle<'row'> diff --git a/packages/ui-table/src/Table/v2/Row/styles.ts b/packages/ui-table/src/Table/v2/Row/styles.ts index 75131b462f..aac43b1fb4 100644 --- a/packages/ui-table/src/Table/v2/Row/styles.ts +++ b/packages/ui-table/src/Table/v2/Row/styles.ts @@ -22,7 +22,7 @@ * SOFTWARE. */ -import type { NewComponentTypes, SharedTokens } from '@instructure/ui-themes' +import type { NewComponentTypes } from '@instructure/ui-themes' import type { TableRowProps, TableRowStyle } from './props' /** @@ -31,18 +31,18 @@ import type { TableRowProps, TableRowStyle } from './props' * --- * Generates the style object from the theme and provided additional information * @param {Object} componentTheme The theme variable object. - * @param {Object} props the props of the component, the style is applied to - * @param {Object} _sharedTokens Shared token object (not used in this component) - * @param {Object} extraArgs the state of the component, the style is applied to + * @param {Object} params the props and Table context values the style depends on * @return {Object} The final style object, which will be used in the component */ const generateStyle = ( componentTheme: ReturnType, - props: TableRowProps, - _sharedTokens: SharedTokens, - extraArgs: { isStacked: boolean; hover: boolean } + params: { + isStacked: boolean + hover: boolean + setHoverStateTo: TableRowProps['setHoverStateTo'] + } ): TableRowStyle => { - const { setHoverStateTo } = props + const { isStacked, hover, setHoverStateTo } = params const hoverStyles = { borderLeftColor: componentTheme.hoverBorderColor, @@ -62,13 +62,13 @@ const generateStyle = ( borderBottomWidth: '0.0625rem', borderBottomColor: componentTheme.borderColor, - ...((setHoverStateTo ?? extraArgs.hover) && { + ...((setHoverStateTo ?? hover) && { borderLeft: '0.1875rem solid transparent', borderRight: '0.1875rem solid transparent', ...(setHoverStateTo === true ? hoverStyles : { '&:hover': hoverStyles }) }), - ...(extraArgs.isStacked && { + ...(isStacked && { padding: `${componentTheme.paddingVertical} ${componentTheme.paddingHorizontal}` }) } diff --git a/regression-test/src/app/breadcrumb/page.tsx b/regression-test/src/app/breadcrumb/page.tsx index 228a0373c1..8f13ad32ae 100644 --- a/regression-test/src/app/breadcrumb/page.tsx +++ b/regression-test/src/app/breadcrumb/page.tsx @@ -90,6 +90,15 @@ export default function BreadcrumbPage() { New Question + + + English literature 204 + } href="#"> + Second term modules + + Current lesson overview + + ) }