Skip to content
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.

Sign up for GitHub

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

Handling central text overflow and adding tooltip for donut charts #26192

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Handling central text overflow and adding tooltip for donut charts #26192

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7e3a3c2
Handling text overflow and adding tooltip for center texts of donut c…
srmukher Jan 5, 2023
42d2636
Wrapping followed by truncation of inner text
srmukher Jan 19, 2023
322ca58
Making the tooltip part of the donut chart svg instead of the body
srmukher Feb 6, 2023
62a5b7f
Adding interactive tests for dynamic donut chart inner text
srmukher Feb 6, 2023
d1d70a7
Merge branch 'master' of https://github.com/srmukher/fluentui into us…
srmukher Feb 6, 2023
66c1fa7
Resolving comments
srmukher Feb 23, 2023
2457a45
Merging master
srmukher Apr 13, 2023
8ce67c1
Merge branch 'master' of https://github.com/srmukher/fluentui into us…
srmukher Apr 13, 2023
60789fd
Merging master
srmukher Aug 3, 2023
b9e2c85
Added few more changes
srmukher Aug 3, 2023
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
7 changes: 7 additions & 0 deletions change/@fluentui-react-charting-4722a315-f7b2-4e91-9f26-23585f066b78.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Bug Fix",
"packageName": "@fluentui/react-charting",
"email": "[email protected]",
"dependentChangeType": "patch"
}
27 changes: 27 additions & 0 deletions packages/react-charting/src/components/DonutChart/Arc/Arc.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,32 @@ export const getStyles = (props: IArcProps): IArcStyles => {
},
},
},
tooltip: {
...theme.fonts.medium,
display: 'flex',
flexDirection: 'column',
padding: '8px',
position: 'absolute',
textAlign: 'center',
top: '0px',
background: theme.semanticColors.bodyBackground,
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a default theme always available

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the theme is coming as a prop from the parent component, which is a required property

borderRadius: '2px',
pointerEvents: 'none',
},
nodeTextContainer: {
selectors: {
text: {
selectors: {
[HighContrastSelectorBlack]: {
fill: 'rgb(179, 179, 179)',
},
},
},
},
marginTop: '4px',
marginLeft: '8px',
marginBottom: '4px',
marginRight: '8px',
},
};
};
96 changes: 90 additions & 6 deletions packages/react-charting/src/components/DonutChart/Arc/Arc.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import * as React from 'react';
import * as shape from 'd3-shape';
import { classNamesFunction } from '@fluentui/react/lib/Utilities';
import { getStyles } from './Arc.styles';
import { IChartDataPoint } from '../index';
import { IArcProps, IArcStyles } from './index';
import { wrapTextInsideDonut } from '../../../utilities/index';
import { select as d3Select } from 'd3-selection';
import { IProcessedStyleSet } from '../../../Styling';

export interface IArcState {
isCalloutVisible?: boolean;
Expand All @@ -19,6 +22,8 @@ export class Arc extends React.Component<IArcProps, IArcState> {

public state: {} = {};

private _tooltip: any;
private _classNames: IProcessedStyleSet<IArcStyles>;
private currentRef = React.createRef<SVGPathElement>();

public static getDerivedStateFromProps(nextProps: Readonly<IArcProps>): Partial<IArcState> | null {
Expand All @@ -33,24 +38,33 @@ export class Arc extends React.Component<IArcProps, IArcState> {
public render(): JSX.Element {
const { arc, href, focusedArcId } = this.props;
const getClassNames = classNamesFunction<IArcProps, IArcStyles>();
const classNames = getClassNames(getStyles, {
this._classNames = getClassNames(getStyles, {
color: this.props.color,
href: href!,
theme: this.props.theme!,
});
const id = this.props.uniqText! + this.props.data!.data.legend!.replace(/\s+/, '') + this.props.data!.data.data;
const opacity: number =
this.props.activeArc === this.props.data!.data.legend || this.props.activeArc === '' ? 1 : 0.1;
let truncatedText: string = '';
if (this.props.valueInsideDonut !== null && this.props.valueInsideDonut !== undefined) {
truncatedText = this._getTruncatedText(
this.props.valueInsideDonut!.toString(),
this.props.innerRadius! * 2 - TEXT_PADDING,
);
}
const isTruncated: boolean = truncatedText.slice(-3) === '...';

return (
<g ref={this.currentRef}>
{!!focusedArcId && focusedArcId === id && (
<path id={id + 'focusRing'} d={arc(this.props.focusData)} className={classNames.focusRing} />
<path id={id + 'focusRing'} d={arc(this.props.focusData)} className={this._classNames.focusRing} />
)}
<path
id={id}
d={arc(this.props.data)}
onFocus={this._onFocus.bind(this, this.props.data!.data, id)}
className={classNames.root}
className={this._classNames.root}
data-is-focusable={true}
onMouseOver={this._hoverOn.bind(this, this.props.data!.data)}
onMouseMove={this._hoverOn.bind(this, this.props.data!.data)}
Expand All @@ -61,13 +75,30 @@ export class Arc extends React.Component<IArcProps, IArcState> {
aria-label={this._getAriaLabel()}
role="img"
/>
<text textAnchor={'middle'} className={classNames.insideDonutString} y={5}>
{this.props.valueInsideDonut!}
</text>
<g className={this._classNames.nodeTextContainer}>
<text
textAnchor={'middle'}
className={this._classNames.insideDonutString}
y={5}
id={'Donut_center_text'}
Copy link
Contributor

Choose a reason for hiding this comment

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

dont hardcode ids

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the wrapping utility has dependency on this Id, need to keep this Id hardcoded

onMouseOver={this._showTooltip.bind(this, this.props.valueInsideDonut!, isTruncated)}
onMouseOut={this._hideTooltip}
>
{truncatedText}
</text>
</g>
</g>
);
}

public componentDidMount(): void {
this._tooltip = d3Select('body')
.append('div')
.attr('id', 'Donut_tooltip')
.attr('class', this._classNames.tooltip!)
.style('opacity', 0);
}

public componentDidUpdate(): void {
const { href } = this.props;
const getClassNames = classNamesFunction<IArcProps, IArcStyles>();
Expand All @@ -80,6 +111,59 @@ export class Arc extends React.Component<IArcProps, IArcState> {
wrapTextInsideDonut(classNames.insideDonutString, this.props.innerRadius! * 2 - TEXT_PADDING);
}

private _getTruncatedText(text: string, maxWidth: number): string {
Copy link
Contributor

@AtishayMsft AtishayMsft Jan 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

How does this work along with wrapTextInsideDonut, if wrapping and truncation both are enabled on the same text

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrapTextInsideDonut ensures that the words are placed in a new line if the current line length exceeds the maximum allowable width inside the donut. However, if a single word itself (even when placed in a new line) exceeds the maxWidth, it does not truncate the word. Thus, _getTruncatedText() determines if truncation is required for a single word or a group of words when inner text exceeds the maximum allowable width.

const words = text.split(/\s+/).reverse();
let word: string = '';
const line: string[] = [];
let truncatedText = text;
const tspan = d3Select('#Donut_center_text').text(null).append('tspan');
let ellipsisLength = 0;

if (tspan.node() !== null && tspan.node() !== undefined) {
// Determine the ellipsis length for word truncation.
tspan.text('...');
ellipsisLength = tspan.node()!.getComputedTextLength();
tspan.text(null);
truncatedText = '';

while ((word = words.pop()!)) {
line.push(word);
tspan.text(line.join(' ') + ' ');
Copy link
Contributor

Choose a reason for hiding this comment

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

You are truncating here without wrapping. First try to solve via wrapping and then by truncation. And lets discuss what all possible scenarios are there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the change to wrap first and then truncate. Added 7 corner cases too that were handled.

Copy link
Contributor

Choose a reason for hiding this comment

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

In case 5 and case 6, you mentioned you are truncating vertically and horizontally, but you are truncating it only once. Can you clarify on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Atishay, this change is now outdated and I have moved it to the utilities.ts in wrapTextInsideDonut(). After wrapping, if the text exceeds either horizontally (exceeds the central width) or vertically (if it goes beyond the inner circle vertically), then we are adding the ellipsis and breaking. If we want to continue truncation even after the text is truncated one way, we will need to add ellipsis both horizontally and vertically might not be a good experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

My comment is on the description that you have provided for case 5 and case 6. Here you are truncating only once and not both ways. Can you update the description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated description.

// Determine if truncation is required. If yes, append the ellipsis and break.
if (tspan.node()!.getComputedTextLength() > maxWidth - ellipsisLength && line.length) {
line.pop();
while (tspan.node()!.getComputedTextLength() > maxWidth - ellipsisLength) {
word = word.slice(0, -1);
tspan.text(word);
}
word += '...';
line.push(word);
tspan.text(line.join(' '));
break;
}
}
truncatedText = tspan.text();
tspan.text(null);
}
return truncatedText;
}

private _showTooltip = (text: string | number, checkTruncated: boolean, evt: any) => {
if (checkTruncated && text !== null && text !== undefined && this._tooltip) {
this._tooltip.style('opacity', 0.9);
this._tooltip
.html(text)
.style('left', evt.pageX + 'px')
.style('top', evt.pageY - 28 + 'px');
}
};

private _hideTooltip = () => {
if (this._tooltip) {
this._tooltip.style('opacity', 0);
}
};

private _onFocus(data: IChartDataPoint, id: string): void {
this.props.onFocusCallback!(data, id, this.currentRef.current);
}
Expand Down
10 changes: 10 additions & 0 deletions packages/react-charting/src/components/DonutChart/Arc/Arc.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,14 @@ export interface IArcStyles {
* styles for the focus
*/
focusRing: IStyle;

/**
* Style for tool tip
*/
tooltip?: IStyle;

/**
* Style for overflow center text container
*/
nodeTextContainer?: IStyle;
}
Loading