Skip to content

Commit

Permalink
fix: pageheader subtitle truncation visibility (#6551)
Browse files Browse the repository at this point in the history
* fix: pageheader subtitle truncation visibility

* fix: pageheader useoverflow hook

* chore: copyright

* fix: refactor overflow utils and logic

* fix: overflow check refactor

* fix: use one ref instead

* fix: remove unnecessary title attribute

* fix: remove console log oops

* fix: add testing

* fix: description

---------

Co-authored-by: Amal K Joy <[email protected]>
  • Loading branch information
davidmenendez and amal-k-joy authored Dec 13, 2024
1 parent 36bd4f9 commit 26394dd
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -538,14 +538,8 @@ $right-section-alt-width: 100% - $left-section-alt-width;
}

.#{$block-class}__subtitle-row {
display: -webkit-box;
overflow: hidden;
max-width: 100%;
margin-top: $spacing-03;

-webkit-box-orient: vertical;
-webkit-line-clamp: 2;

@include breakpoint-up('md') {
max-width: $left-section-std-width;
}
Expand All @@ -559,6 +553,24 @@ $right-section-alt-width: 100% - $left-section-alt-width;
@include type.type-style('body-01');
}

.#{$block-class}__subtitle-tooltip .#{$carbon-prefix}--definition-term {
border-bottom: 0;
letter-spacing: inherit;
}

// overwrites the existing styles to make the popover bigger because in some cases the narrow space can be too constricting for the header
.#{$block-class}__subtitle-tooltip
.#{$carbon-prefix}--popover-content.#{$carbon-prefix}--definition-tooltip {
max-inline-size: fit-content;
}

.#{$block-class}__subtitle-text {
display: -webkit-box;
overflow: hidden;
-webkit-box-orient: vertical;
-webkit-line-clamp: 2;
}

.#{$block-class}__available-row {
@include type.type-style('body-01');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ const pageActionsOverflowLabel = 'Page actions...';

const subtitle = 'Optional subtitle if necessary';
const longSubtitle =
'Optional subtitle if necessary, which is very long in this case, but will need to be handled somehow. It just keeps going on and on and on and on and on.';
'Optional subtitle if necessary, which is very long in this case, but will need to be handled somehow. It just keeps going on and on and on and on and on and on and on and on and on and on and on.';
const demoSubtitle = 'This report details the monthly authentication failures';

const dummyPageContent = (
Expand Down
27 changes: 23 additions & 4 deletions packages/ibm-products/src/components/PageHeader/PageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
usePrefix,
ButtonProps,
PopoverAlignment,
DefinitionTooltip,
} from '@carbon/react';
import { TagProps } from '@carbon/react/lib/components/Tag/Tag';
import React, {
Expand Down Expand Up @@ -51,6 +52,7 @@ import cx from 'classnames';
import { getDevtoolsProps } from '../../global/js/utils/devtools';
import { pkg } from '../../settings';
import { useResizeObserver } from '../../global/js/hooks/useResizeObserver';
import { checkHeightOverflow } from '../../global/js/utils/checkForOverflow';

const componentName = 'PageHeader';

Expand Down Expand Up @@ -901,12 +903,20 @@ export let PageHeader = React.forwardRef(

const displayedBreadcrumbs = getBreadcrumbs();

const subtitleRef = useRef<HTMLSpanElement>(null);
const isOverflowing = checkHeightOverflow(subtitleRef.current);
const subtitleContent = (
<span ref={subtitleRef} className={`${blockClass}__subtitle-text`}>
{subtitle}
</span>
);

return (
<>
<div
className={`${blockClass}--offset-top-measuring-element`}
ref={offsetTopMeasuringRef}
></div>
/>
<section
{...rest}
className={cx([
Expand Down Expand Up @@ -1035,13 +1045,22 @@ export let PageHeader = React.forwardRef(
</Row>
) : null}

{subtitle ? (
{subtitle && (
<Row className={`${blockClass}__subtitle-row`}>
<Column className={`${blockClass}__subtitle`}>
{subtitle}
{isOverflowing ? (
<DefinitionTooltip
definition={subtitle}
className={`${blockClass}__subtitle-tooltip`}
>
{subtitleContent}
</DefinitionTooltip>
) : (
subtitleContent
)}
</Column>
</Row>
) : null}
)}

{children ? (
<Row className={`${blockClass}__available-row`}>
Expand Down
42 changes: 12 additions & 30 deletions packages/ibm-products/src/components/PageHeader/PageHeaderTitle.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/

import React, { useLayoutEffect, useRef, useState } from 'react';
import React, { useRef } from 'react';
import PropTypes from 'prop-types';
import cx from 'classnames';
import { DefinitionTooltip, SkeletonText } from '@carbon/react';
import { EditInPlace } from '../EditInPlace';
import { checkWidthOverflow } from '../../global/js/utils/checkForOverflow';

/**
*
Expand Down Expand Up @@ -39,25 +40,8 @@ export const PageHeaderTitle = ({ blockClass, hasBreadcrumbRow, title }) => {
let titleText;
let isEditable = !!onSave;

const [isEllipsisApplied, setIsEllipsisApplied] = useState();
const longTitleRef = useRef(undefined);
const titleRef = useRef(undefined);

useLayoutEffect(() => {
setIsEllipsisApplied(isEllipsisActive());
}, [longTitleRef, titleRef, title]);

const isEllipsisActive = () => {
if (longTitleRef.current) {
return (
longTitleRef.current?.offsetWidth < longTitleRef.current?.scrollWidth
);
} else if (titleRef.current) {
return titleRef.current?.offsetWidth < titleRef.current?.scrollWidth;
}

return false;
};
const titleRef = useRef();
const isEllipsisApplied = checkWidthOverflow(titleRef.current);

if (text || !content) {
if (text === undefined && typeof title === 'string') {
Expand All @@ -66,6 +50,12 @@ export const PageHeaderTitle = ({ blockClass, hasBreadcrumbRow, title }) => {
}
const TitleIcon = icon;

const titleContent = (
<span ref={titleRef} className={`${blockClass}__titleText`}>
{text}
</span>
);

titleInnards = (
<>
{icon && !loading ? (
Expand Down Expand Up @@ -97,18 +87,10 @@ export const PageHeaderTitle = ({ blockClass, hasBreadcrumbRow, title }) => {
definition={text}
className={`${blockClass}__tooltip`}
>
<span ref={longTitleRef} className={`${blockClass}__titleText`}>
{text}
</span>
{titleContent}
</DefinitionTooltip>
) : (
<span
ref={titleRef}
className={`${blockClass}__titleText`}
title={!loading ? asText : null}
>
{text}
</span>
titleContent
)}
</>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Copyright IBM Corp. 2024
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
*/

import { checkWidthOverflow, checkHeightOverflow } from '../checkForOverflow';

const normalElm = {
offsetWidth: 200,
scrollWidth: 100,
offsetHeight: 200,
scrollHeight: 100,
};

const overflowElm = {
offsetWidth: 100,
scrollWidth: 200,
offsetHeight: 100,
scrollHeight: 200,
};

describe('checkForOverflow', () => {
beforeEach(() => {
jest.resetAllMocks();
});

it('detects width overflow', () => {
expect(checkWidthOverflow(normalElm)).toBe(false);
expect(checkWidthOverflow(overflowElm)).toBe(true);
expect(checkWidthOverflow()).toBe(false);
});

it('detects height overflow', () => {
expect(checkHeightOverflow(normalElm)).toBe(false);
expect(checkHeightOverflow(overflowElm)).toBe(true);
expect(checkHeightOverflow()).toBe(false);
});
});
24 changes: 24 additions & 0 deletions packages/ibm-products/src/global/js/utils/checkForOverflow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//
// Copyright IBM Corp. 2024, 2024
//
// This source code is licensed under the Apache-2.0 license found in the
// LICENSE file in the root directory of this source tree.
//

/**
* used to calculate if a element is overflowing the width or height of an area
*/

export const checkWidthOverflow = (el: HTMLElement | null): boolean => {
if (el) {
return el?.offsetWidth < el?.scrollWidth;
}
return false;
};

export const checkHeightOverflow = (el: HTMLElement | null): boolean => {
if (el) {
return el?.offsetHeight < el?.scrollHeight;
}
return false;
};

0 comments on commit 26394dd

Please sign in to comment.