From 13be4424b57072c25a721bfd8591c8540ef396d6 Mon Sep 17 00:00:00 2001
From: Usama Ansari <115616380+dv-usama-ansari@users.noreply.github.com>
Date: Thu, 19 Dec 2024 14:49:50 +0100
Subject: [PATCH] fix(vis-type: bar): focus facet selector + sort facets
 alphabetically (#665)

* fix: focus facet selector

* fix: lint errors

* fix: focusFacetIndex check

* chore: add sorting to the facets

* chore: add sorting to the facets

---------

Co-authored-by: Holger Stitz <holger.stitz@datavisyn.io>
---
 src/vis/bar/BarChart.tsx                      |  9 +++++----
 src/vis/bar/components/FocusFacetSelector.tsx | 12 +++++++-----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/vis/bar/BarChart.tsx b/src/vis/bar/BarChart.tsx
index 85f517283..5b51b7963 100644
--- a/src/vis/bar/BarChart.tsx
+++ b/src/vis/bar/BarChart.tsx
@@ -108,7 +108,8 @@ export function BarChart({
   const allUniqueFacetVals = React.useMemo(() => {
     const set = new Set();
     barData?.facetsColVals?.resolvedValues.forEach((v) => set.add(getLabelOrUnknown(v.val)));
-    return [...set] as string[];
+    const uniqueFacetValues = [...set] as string[];
+    return uniqueFacetValues.sort((a, b) => (a === NAN_REPLACEMENT ? 1 : b === NAN_REPLACEMENT ? -1 : a && b ? a.localeCompare(b) : 0));
   }, [barData?.facetsColVals?.resolvedValues]);
 
   const filteredUniqueFacetVals = React.useMemo(() => {
@@ -116,7 +117,7 @@ export function BarChart({
       typeof config?.focusFacetIndex === 'number' && config?.focusFacetIndex < allUniqueFacetVals.length
         ? ([allUniqueFacetVals[config?.focusFacetIndex]] as string[])
         : allUniqueFacetVals;
-    return unsorted.sort((a, b) => (a === NAN_REPLACEMENT || b === NAN_REPLACEMENT ? 1 : a && b ? a.localeCompare(b) : 0));
+    return unsorted.sort((a, b) => (a === NAN_REPLACEMENT ? 1 : b === NAN_REPLACEMENT ? -1 : a && b ? a.localeCompare(b) : 0));
   }, [allUniqueFacetVals, config?.focusFacetIndex]);
 
   const groupColorScale = React.useMemo(() => {
@@ -187,8 +188,8 @@ export function BarChart({
   }, [aggregatedDataMap?.facets, config]);
 
   const shouldRenderFacets = React.useMemo(
-    () => Boolean(config?.facets && barData?.facetsColVals && filteredUniqueFacetVals.length === Object.keys(chartHeightMap).length),
-    [config?.facets, barData?.facetsColVals, filteredUniqueFacetVals.length, chartHeightMap],
+    () => Boolean(config?.facets && barData?.facetsColVals && (config?.focusFacetIndex !== undefined || config?.focusFacetIndex !== null)),
+    [config?.facets, barData?.facetsColVals, config?.focusFacetIndex],
   );
 
   const isGroupedByNumerical = React.useMemo(() => barData?.groupColVals?.type === EColumnTypes.NUMERICAL, [barData?.groupColVals?.type]);
diff --git a/src/vis/bar/components/FocusFacetSelector.tsx b/src/vis/bar/components/FocusFacetSelector.tsx
index a5e34408b..44e27eb96 100644
--- a/src/vis/bar/components/FocusFacetSelector.tsx
+++ b/src/vis/bar/components/FocusFacetSelector.tsx
@@ -6,6 +6,8 @@ import type { ICommonVisProps } from '../../interfaces';
 import { IBarConfig } from '../interfaces';
 
 export function FocusFacetSelector({ config, setConfig, facets }: Pick<ICommonVisProps<IBarConfig>, 'config' | 'setConfig'> & { facets: string[] }) {
+  const isFacetFocused: boolean = React.useMemo(() => config?.focusFacetIndex !== null && config?.focusFacetIndex !== undefined, [config?.focusFacetIndex]);
+
   if (!config?.facets && facets.length === 0) {
     return null;
   }
@@ -17,17 +19,17 @@ export function FocusFacetSelector({ config, setConfig, facets }: Pick<ICommonVi
           key={`focusFacetSelect_${config.focusFacetIndex ?? 0}`}
           placeholder="Select a focus facet"
           data={facets}
-          value={facets[config.focusFacetIndex ?? 0] || ''}
+          value={isFacetFocused ? facets[config.focusFacetIndex ?? 0] : ''}
           onChange={(value) => {
             setConfig?.({ ...config, focusFacetIndex: typeof value === 'string' ? facets.indexOf(value) : value });
           }}
           clearable
         />
-        <Tooltip label="Previous facet" position="top" withArrow>
+        <Tooltip label="Previous facet" position="top" hidden={!isFacetFocused} withArrow withinPortal>
           <ActionIcon
             color="dvGray"
             variant="subtle"
-            disabled={(config.focusFacetIndex ?? 0) === null}
+            disabled={!isFacetFocused}
             onClick={() => {
               setConfig?.({ ...config, focusFacetIndex: ((config.focusFacetIndex ?? 0) - 1 + facets.length) % facets.length });
             }}
@@ -35,11 +37,11 @@ export function FocusFacetSelector({ config, setConfig, facets }: Pick<ICommonVi
             <FontAwesomeIcon icon={faChevronLeft} />
           </ActionIcon>
         </Tooltip>
-        <Tooltip label="Next facet" position="top" withArrow>
+        <Tooltip label="Next facet" position="top" hidden={!isFacetFocused} withArrow withinPortal>
           <ActionIcon
             color="dvGray"
             variant="subtle"
-            disabled={(config.focusFacetIndex ?? 0) === null}
+            disabled={!isFacetFocused}
             onClick={() => {
               setConfig?.({ ...config, focusFacetIndex: ((config.focusFacetIndex ?? 0) + 1) % facets.length });
             }}