From e23bb9a8c281e9ab318b4aced99eb9ff3a4b1b3d Mon Sep 17 00:00:00 2001 From: Shubhabrata Ghosh Date: Tue, 14 May 2024 23:28:58 +0530 Subject: [PATCH 1/4] Fix order by sorting and mapping indices --- .../SankeyChart/SankeyChart.base.tsx | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx b/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx index 6f99b53402e277..b3d7b5a0acd229 100644 --- a/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx +++ b/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx @@ -759,6 +759,31 @@ export class SankeyChartBase extends React.Component { + if (a.x0 !== b.x0) { + return a.x0! - b.x0!; + } + return a.y0! - b.y0!; + }); + nodes.forEach((item: SNode, index) => { + nodeLinkDomOrderArray.push({ layer: item.layer, type: 'Node', index: index }); + }); + links.sort((a: SLink, b: SLink) => { + if (a.source.x0 !== b.source.x0) { + return a.source.x0! - b.source.x0!; + } + return a.y0! - b.y0!; + }); + links.forEach((item: SLink, index) => { + nodeLinkDomOrderArray.push({ layer: item.source.layer, type: 'Link', index: index }); + }); + nodeLinkDomOrderArray.sort((a, b) => { + if (a.layer !== b.layer) { + return a.layer - b.layer; + } + return b.type - a.type; + }); // NOTE: I don't love this approach to caching the "select" result. Is it still valid from render-to-render? // although local testing seems to indicate so, I do not trust that React will always support that instance. // It might be better to perform this `fetch` within the `_showTooltip` and `_hideTooltip` methods. @@ -798,10 +823,18 @@ export class SankeyChartBase extends React.Component - - {linkData} - - {nodeData} + {nodeLinkDomOrderArray.map(item => { + if (item.type === 'Node') { + return {nodeData![item.index]}; + } + + return ( + // eslint-disable-next-line react/jsx-key + + {linkData![item.index]} + + ); + })} {calloutProps.isCalloutVisible && ( Date: Fri, 24 May 2024 00:29:44 +0530 Subject: [PATCH 2/4] update focus order in sankey chart for vertical navigation --- .../SankeyChart/SankeyChart.base.tsx | 39 +- .../__snapshots__/SankeyChart.test.tsx.snap | 9841 ++++++++++------- .../SankeyChartRTL.test.tsx.snap | 621 +- 3 files changed, 6458 insertions(+), 4043 deletions(-) diff --git a/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx b/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx index cdd19349d3c49b..87b4bca5603068 100644 --- a/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx +++ b/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx @@ -759,7 +759,7 @@ export class SankeyChartBase extends React.Component { if (a.x0 !== b.x0) { return a.x0! - b.x0!; @@ -767,23 +767,33 @@ export class SankeyChartBase extends React.Component { - nodeLinkDomOrderArray.push({ layer: item.layer, type: 'Node', index: index }); + nodeLinkDomOrderArray.push({ layer: item.layer!, type: 'node', index: index }); }); links.sort((a: SLink, b: SLink) => { - if (a.source.x0 !== b.source.x0) { - return a.source.x0! - b.source.x0!; + const asx0 = (a.source as SNode).x0; + const bsx0 = (b.source as SNode).x0; + if (asx0 !== bsx0) { + return asx0! - bsx0!; } return a.y0! - b.y0!; }); links.forEach((item: SLink, index) => { - nodeLinkDomOrderArray.push({ layer: item.source.layer, type: 'Link', index: index }); + nodeLinkDomOrderArray.push({ layer: (item.source as SNode).layer!, type: 'link', index: index }); }); nodeLinkDomOrderArray.sort((a, b) => { if (a.layer !== b.layer) { return a.layer - b.layer; } - return b.type - a.type; + + if (a.type > b.type) { + return -1; + } + if (a.type < b.type) { + return 1; + } + return 0; }); + // NOTE: I don't love this approach to caching the "select" result. Is it still valid from render-to-render? // although local testing seems to indicate so, I do not trust that React will always support that instance. // It might be better to perform this `fetch` within the `_showTooltip` and `_hideTooltip` methods. @@ -817,16 +827,23 @@ export class SankeyChartBase extends React.Component (this.chartContainer = rootElem)} onMouseLeave={this._onCloseCallout} > - + {nodeLinkDomOrderArray.map(item => { - if (item.type === 'Node') { - return {nodeData![item.index]}; + if (item.type === 'node') { + return ( + + {nodeData![item.index]} + + ); } return ( - // eslint-disable-next-line react/jsx-key - + {linkData![item.index]} ); diff --git a/packages/react-charting/src/components/SankeyChart/__snapshots__/SankeyChart.test.tsx.snap b/packages/react-charting/src/components/SankeyChart/__snapshots__/SankeyChart.test.tsx.snap index 02ec74705abcec..c2bc3e6c6f570c 100644 --- a/packages/react-charting/src/components/SankeyChart/__snapshots__/SankeyChart.test.tsx.snap +++ b/packages/react-charting/src/components/SankeyChart/__snapshots__/SankeyChart.test.tsx.snap @@ -36,6 +36,85 @@ exports[`Sankey Chart snapShot testing number formatting renders Sankey correctl id="sankeyChart2" width={864} > + + + + + + + + + + + + + + + + + - - - - - - - - + + + - - - - + - - - - - + + + + - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - First + Fourth - 1.33B + 1B + + - - - - - - - - - - - - - - - - - - - - - - - - - - - + + @@ -701,6 +854,17 @@ exports[`Sankey Chart snapShot testing number formatting renders Sankey correctl y={414.9417673221558} /> + + @@ -768,124 +932,98 @@ exports[`Sankey Chart snapShot testing number formatting renders Sankey correctl className= { - fill: #ffffff; - stroke-width: 3px; - stroke: #000000; + fill: #F5F5F5; } @media screen and (-ms-high-contrast: white-on-black), screen and (forced-colors: active) and (prefers-color-scheme: dark){& { fill: #000000; } - strokeOpacity={1} > - - - - - - - - + - - - - + - - - - - - - - - + + + + + + - - - - - - - - - - First + Second - 100% + 60% + + - Second + Third - 60% + 40% + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - Third + Fourth 40% - - - + - - - - - - + { + fill: #F5F5F5; + } + @media screen and (-ms-high-contrast: white-on-black), screen and (forced-colors: active) and (prefers-color-scheme: dark){& { + fill: #000000; + } + > @@ -1384,159 +1662,352 @@ exports[`Sankey Chart snapShot testing renders Sankey correctly 1`] = ` className= { - fill: #ffffff; - stroke-width: 3px; - stroke: #000000; + fill: #F5F5F5; } @media screen and (-ms-high-contrast: white-on-black), screen and (forced-colors: active) and (prefers-color-scheme: dark){& { fill: #000000; } - strokeOpacity={1} > - - - - - - - - + - - - - + - - - - - - - - - + + + + + + - - - - + - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - 192.168.42.72 + inbox - 80 + 130 + + - 172.152.48.13 + Junk Folder - 78 + 69 + + - - - - 192.564.10.2 + Deleted Folder - - + + + + + + + + + - - + + + + + + + + + - - + + + + + + + + + - + + - - - - - + + + + + + + - Junk Folder + Clicked - 69 + 30 + + - Deleted Folder + Opened + + - Clicked + No further action required - 30 + 62 + + + + +`; + +exports[`Sankey Chart snapShot testing renders Sankey correctly on providing nodecolors and border colors 1`] = ` +
+
+ + - Opened + 192.168.42.72 - 55 + 80 + + - No further action required + 172.152.48.13 - 62 + 78 - -
-
-`; - -exports[`Sankey Chart snapShot testing renders Sankey correctly on providing nodecolors and border colors 1`] = ` -
-
- - - - - - - - - + - - - - - - - - + + + - - - - - - - - + + + - - - - + + + + - - + + + + + + + + + + + + + + + + + + + + + + - 192.168.42.72 + inbox - 80 + 130 + + - 172.152.48.13 + Junk Folder - 78 + 69 + + - - - - 192.564.10.2 + Deleted Folder - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - + + + + + + + + + + + - inbox + Clicked - 130 + 30 + + - Junk Folder + Opened - 69 + 55 + + - Deleted Folder + No further action required + + +
+
+`; + +exports[`Sankey Chart snapShot testing renders Sankey correctly with supplied resource strings 1`] = ` +
+
+ + - Clicked + First - 30 + 10 + + + + + + + + + + + + + - Opened + Second - 55 + 10 + + +
+
+`; + +exports[`SankeyChart - Min Height of Node Test renders Sankey correctly on providing height less than onepercent of total height 1`] = ` +
+
+ + + + + + + - + + - - - - - + { + fill: #F5F5F5; + } + @media screen and (-ms-high-contrast: white-on-black), screen and (forced-colors: active) and (prefers-color-scheme: dark){& { + fill: #000000; + } + > + + - -
-
-`; - -exports[`Sankey Chart snapShot testing renders Sankey correctly with supplied resource strings 1`] = ` -
-
- - - - - - - - - + @@ -3759,24 +4914,24 @@ exports[`Sankey Chart snapShot testing renders Sankey correctly with supplied re } > - First + node4 - 10 + 900 + + - Second + node5 - -
-
-`; - -exports[`SankeyChart - Min Height of Node Test renders Sankey correctly on providing height less than onepercent of total height 1`] = ` -
-
- + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -4411,6 +5386,17 @@ exports[`SankeyChart - Min Height of Node Test renders Sankey correctly on provi y={47.95153640302901} /> + + @@ -4521,6 +5507,117 @@ exports[`SankeyChart - Min Height of Node Test renders Sankey correctly on provi id="sankeyChart2" width={864} > + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+`; + +exports[`SankeyChart - mouse events Should render callout correctly on mouseover when height of node is less than 24px 1`] = ` +
+
+ + + + + + + + + + + + + + + + + + + + + + + - -
-
-`; - -exports[`SankeyChart - mouse events Should render callout correctly on mouseover when height of node is less than 24px 1`] = ` -
-
- + + @@ -4992,161 +6409,27 @@ exports[`SankeyChart - mouse events Should render callout correctly on mouseover strokeWidth="2" /> + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + - 192.168.42.72 + inbox - 80 + 130 + + - - - - + textAnchor="start" + x={393.5} + y={270.8534770168133} + > + 69 + + + + - 192.564.10.2 + Deleted Folder + + - + + + + + + + + - + + + + + + + + - + + + + + + - - - - - + + + + - 130 - - + + + + + + + + + - Deleted Folder + Opened + + + + + +
+
+`; + +exports[`SankeyChart - mouse events Should render correctly on link mouseover 1`] = ` +
+
+ + + + - No further action required + 172.152.48.13 - 62 + 78 - - -
-
-`; - -exports[`SankeyChart - mouse events Should render correctly on link mouseover 1`] = ` -
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + { + fill: #F5F5F5; + } + @media screen and (-ms-high-contrast: white-on-black), screen and (forced-colors: active) and (prefers-color-scheme: dark){& { + fill: #000000; + } + key="2" + > - - - - - - - + + - - - - - - - + + - - - - - - - + + + + + + + + - - - - - - - + + + + + + + + - - - - - - - - - - - - - - - - - - + fill: #ffffff; + stroke-width: 3px; + stroke: #000000; + } + @media screen and (-ms-high-contrast: white-on-black), screen and (forced-colors: active) and (prefers-color-scheme: dark){& { + fill: #000000; + } + key="4-7" + strokeOpacity={1} + > - + + + + + + + + - + + + + + + - - - - - - + + - + + + + + + + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - + + + + + + + + - - - - - - - - - - - + - - - - - + + + + + + - - - - - - - + + - - - - - - - + + - - - - - - - + + + + + + + + - - - - - - - + + + + + + + + - - - - - - - - - - - - - - - - - - + } + key="4-7" + strokeOpacity={1} + > - + + + + + + + + - + + + + + + - - - - - - + + - + + + + + + + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - + - - - - - - - - - + + + + - - - - - - - + + + + + + + + - - - - - - - + + - - - - - - - + + - - - - - - - + + + + + + + + - - - - - - - + + + + + + + + - - - - - - - - - - - - - - - - - - - + + + + + + + + - + + + + + + - - - - - - + + - + + + + + + + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - + + @@ -250,6 +183,109 @@ exports[`Sankey bar chart rendering Should render the Sankey chart with string n + + + + + + + + + + + + + + + + + + + + + + + + @@ -314,6 +350,17 @@ exports[`Sankey bar chart rendering Should render the Sankey chart with string n + + @@ -432,84 +479,6 @@ exports[`Sankey chart - Theme Should reflect theme change 1`] = ` id="sankeyChart5" width="864" > - - - - - - - - - - - - - - - - - - - - + + @@ -644,10 +624,113 @@ exports[`Sankey chart - Theme Should reflect theme change 1`] = ` x="48" y="281.53846153846155" > - 50 - - + 50 + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -712,6 +795,17 @@ exports[`Sankey chart - Theme Should reflect theme change 1`] = ` + + @@ -830,84 +924,6 @@ exports[`Sankey chart rendering Should re-render the Sankey chart with data 2`] id="sankeyChart2" width="864" > - - - - - - - - - - - - - - - - - - - - + + @@ -1046,6 +1073,109 @@ exports[`Sankey chart rendering Should re-render the Sankey chart with data 2`] + + + + + + + + + + + + + + + + + + + + + + + + @@ -1110,6 +1240,17 @@ exports[`Sankey chart rendering Should re-render the Sankey chart with data 2`] + + From 4c5cb9f9f76621873a9a0b424207c71d88c4d0d8 Mon Sep 17 00:00:00 2001 From: Kumar Kshitij Date: Fri, 24 May 2024 00:30:37 +0530 Subject: [PATCH 3/4] add change file --- ...eact-charting-53e6e8eb-6b93-49b1-8419-7701781c49a0.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-charting-53e6e8eb-6b93-49b1-8419-7701781c49a0.json diff --git a/change/@fluentui-react-charting-53e6e8eb-6b93-49b1-8419-7701781c49a0.json b/change/@fluentui-react-charting-53e6e8eb-6b93-49b1-8419-7701781c49a0.json new file mode 100644 index 00000000000000..63fa2409a40ea3 --- /dev/null +++ b/change/@fluentui-react-charting-53e6e8eb-6b93-49b1-8419-7701781c49a0.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Update focus order in sankey chart for vertical navigation", + "packageName": "@fluentui/react-charting", + "email": "kumarkshitij@microsoft.com", + "dependentChangeType": "patch" +} From cb2d3225dc229ed0fa7ed3055e68146b88a9227a Mon Sep 17 00:00:00 2001 From: Kumar Kshitij Date: Tue, 28 May 2024 14:40:52 +0530 Subject: [PATCH 4/4] add comments --- .../SankeyChart/SankeyChart.base.tsx | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx b/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx index 87b4bca5603068..cdb6006c0618f9 100644 --- a/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx +++ b/packages/react-charting/src/components/SankeyChart/SankeyChart.base.tsx @@ -759,6 +759,11 @@ export class SankeyChartBase extends React.Component { if (a.x0 !== b.x0) { @@ -827,6 +832,12 @@ export class SankeyChartBase extends React.Component (this.chartContainer = rootElem)} onMouseLeave={this._onCloseCallout} > + {/* + - Horizontal navigation has been disabled because the nodes and links are rendered vertically, + causing the left/right arrow keys to move focus up or down to the previous or next sibling element. + - Bidirectional navigation has been disabled because it causes the up/down arrow keys to move the focus + in a non-sequential and erratic manner within a 2D grid. + */} {nodeLinkDomOrderArray.map(item => { @@ -836,17 +847,19 @@ export class SankeyChartBase extends React.Component ); + } else { + return ( + + {linkData![item.index]} + + ); } - - return ( - - {linkData![item.index]} - - ); })} {calloutProps.isCalloutVisible && (