From af974e97cf92a7d3104bdfe7e2b17fb43d9406c3 Mon Sep 17 00:00:00 2001
From: HendrikThePendric <hendrik@dhis2.org>
Date: Mon, 16 Dec 2024 17:01:49 +0100
Subject: [PATCH] fix: update filter-badge implementation so filters can be
 removed on small screen

---
 src/pages/view/FilterBar/FilterBadge.js       | 99 ++++++++++++-------
 .../FilterBar/__tests__/FilterBadge.spec.js   | 19 ++--
 .../FilterBar/styles/FilterBadge.module.css   |  6 ++
 3 files changed, 79 insertions(+), 45 deletions(-)

diff --git a/src/pages/view/FilterBar/FilterBadge.js b/src/pages/view/FilterBar/FilterBadge.js
index c8cfe003d..015c89553 100644
--- a/src/pages/view/FilterBar/FilterBadge.js
+++ b/src/pages/view/FilterBar/FilterBadge.js
@@ -15,13 +15,25 @@ const getFilterValuesText = (values) =>
         ? values[0].name
         : i18n.t('{{count}} selected', { count: values.length })
 
-const FilterBadge = ({ filter, openFilterModal, onRemove }) => {
-    const { isConnected } = useDhis2ConnectionStatus()
-    const { width } = useWindowDimensions()
-    const notAllowed = !isConnected || width <= 480
-    const tooltipContent = !isConnected
-        ? i18n.t('Cannot edit filters while offline')
-        : i18n.t('Cannot edit filters on a small screen')
+const EditFilterButton = ({ tooltipContent, filter, openFilterModal }) => {
+    const buttonText = `${filter.name}: ${getFilterValuesText(filter.values)}`
+
+    if (!tooltipContent) {
+        return (
+            <button
+                data-test="filter-badge-button"
+                className={cx(classes.button, classes.filterButton)}
+                onClick={() =>
+                    openFilterModal({
+                        id: filter.id,
+                        name: filter.name,
+                    })
+                }
+            >
+                {buttonText}
+            </button>
+        )
+    }
 
     return (
         <Tooltip
@@ -30,42 +42,53 @@ const FilterBadge = ({ filter, openFilterModal, onRemove }) => {
             closeDelay={100}
             className={classes.tooltip}
         >
-            {({ onFocus, onBlur, onMouseOver, onMouseOut, ref }) => (
-                <div
-                    className={classes.container}
-                    data-test="dashboard-filter-badge"
-                    onFocus={() => notAllowed && onFocus()}
-                    onBlur={() => notAllowed && onBlur()}
-                    onMouseOver={() => notAllowed && onMouseOver()}
-                    onMouseOut={() => notAllowed && onMouseOut()}
-                    ref={ref}
+            {(props) => (
+                <button
+                    disabled
+                    data-test="filter-badge-button"
+                    className={cx(classes.button, classes.filterButton)}
+                    {...props}
                 >
-                    <button
-                        data-test="filter-badge-button"
-                        className={cx(classes.button, classes.filterButton)}
-                        disabled={notAllowed}
-                        onClick={() =>
-                            openFilterModal({
-                                id: filter.id,
-                                name: filter.name,
-                            })
-                        }
-                    >
-                        {filter.name}: {getFilterValuesText(filter.values)}
-                    </button>
-                    <button
-                        data-test="filter-badge-clear-button"
-                        className={cx(classes.button, classes.removeButton)}
-                        disabled={notAllowed}
-                        onClick={() => onRemove(filter.id)}
-                    >
-                        <IconCross16 />
-                    </button>
-                </div>
+                    {buttonText}
+                </button>
             )}
         </Tooltip>
     )
 }
+EditFilterButton.propTypes = {
+    filter: PropTypes.object,
+    openFilterModal: PropTypes.func,
+    tooltipContent: PropTypes.string,
+}
+
+const FilterBadge = ({ filter, openFilterModal, onRemove }) => {
+    const { isConnected } = useDhis2ConnectionStatus()
+    const { width } = useWindowDimensions()
+    const isEditDisabled = !isConnected || width <= 480
+    const tooltipContent = !isConnected
+        ? i18n.t('Cannot edit filters while offline')
+        : i18n.t('Cannot edit filters on a small screen')
+
+    return (
+        <div className={classes.container} data-test="dashboard-filter-badge">
+            <EditFilterButton
+                filter={filter}
+                openFilterModal={openFilterModal}
+                tooltipContent={isEditDisabled ? tooltipContent : null}
+            />
+            {isConnected && (
+                <button
+                    data-test="filter-badge-clear-button"
+                    className={cx(classes.button, classes.removeButton)}
+                    disabled={!isConnected}
+                    onClick={() => onRemove(filter.id)}
+                >
+                    <IconCross16 />
+                </button>
+            )}
+        </div>
+    )
+}
 
 FilterBadge.propTypes = {
     filter: PropTypes.object.isRequired,
diff --git a/src/pages/view/FilterBar/__tests__/FilterBadge.spec.js b/src/pages/view/FilterBar/__tests__/FilterBadge.spec.js
index fd0d32110..b183c4643 100644
--- a/src/pages/view/FilterBar/__tests__/FilterBadge.spec.js
+++ b/src/pages/view/FilterBar/__tests__/FilterBadge.spec.js
@@ -79,14 +79,16 @@ test('Has enabled buttons when online', () => {
     expect(getByTestId('filter-badge-clear-button')).toBeEnabled()
 })
 
-test('Has disabled buttons when offline', () => {
-    useDhis2ConnectionStatus.mockImplementation(() => ({ isConnected: false }))
+test('Shows only a disabled edit-filter button when offline', () => {
+    useDhis2ConnectionStatus.mockImplementationOnce(() => ({
+        isConnected: false,
+    }))
     const filter = {
         id: 'ponies',
         name: 'Ponies',
         values: [{ name: 'Twilight Sparkle' }],
     }
-    const { getByTestId } = render(
+    const { getByTestId, queryByTestId } = render(
         <Provider store={createMockStore()}>
             <FilterBadge
                 filter={filter}
@@ -97,11 +99,14 @@ test('Has disabled buttons when offline', () => {
         </Provider>
     )
     expect(getByTestId('filter-badge-button')).toBeDisabled()
-    expect(getByTestId('filter-badge-clear-button')).toBeDisabled()
+    expect(queryByTestId('filter-badge-clear-button')).not.toBeInTheDocument()
 })
 
-test('Has disabled buttons when on small screen', () => {
-    useWindowDimensions.mockImplementation(() => ({ width: 440, height: 780 }))
+test('Shows a disabled edit-filter button and enabled clear-filter when on small screen', () => {
+    useWindowDimensions.mockImplementationOnce(() => ({
+        width: 440,
+        height: 780,
+    }))
     const filter = {
         id: 'ponies',
         name: 'Ponies',
@@ -118,5 +123,5 @@ test('Has disabled buttons when on small screen', () => {
         </Provider>
     )
     expect(getByTestId('filter-badge-button')).toBeDisabled()
-    expect(getByTestId('filter-badge-clear-button')).toBeDisabled()
+    expect(getByTestId('filter-badge-clear-button')).toBeEnabled()
 })
diff --git a/src/pages/view/FilterBar/styles/FilterBadge.module.css b/src/pages/view/FilterBar/styles/FilterBadge.module.css
index 3da675679..3b853fc7b 100644
--- a/src/pages/view/FilterBar/styles/FilterBadge.module.css
+++ b/src/pages/view/FilterBar/styles/FilterBadge.module.css
@@ -28,6 +28,12 @@
     border-end-start-radius: 4px;
 }
 
+.filterButton:only-child {
+    padding-inline-end: 6px;
+    border-start-end-radius: 4px;
+    border-end-end-radius: 4px;
+}
+
 .removeButton {
     padding-inline: 4px 6px;
     border-start-end-radius: 4px;