CAUSEWAY-3830: limit scope of bulk toggle to filtered data-rows

apache · Nov 20, 2024 · 7ec3335 · 7ec3335
1 parent 3b6c116
commit 7ec3335
Show file tree

Hide file tree

Showing 19 changed files with 111 additions and 242 deletions.
diff --git a/commons/src/main/java/org/apache/causeway/commons/internal/collections/_Streams.java b/commons/src/main/java/org/apache/causeway/commons/internal/collections/_Streams.java
@@ -18,8 +18,11 @@
  */
 package org.apache.causeway.commons.internal.collections;
 
+import java.util.Comparator;
 import java.util.stream.Stream;
 
+import org.springframework.lang.Nullable;
+
 /**
  * <h1>- internal use only -</h1>
  * <p>
@@ -41,15 +44,15 @@ private _Streams(){}
     /**
      * 3 param variant of {@link Stream#concat(Stream, Stream)}
      */
-    public static <T> Stream<T> concat(Stream<? extends T> a, Stream<? extends T> b, Stream<? extends T> c) {
+    public static <T> Stream<T> concat(final Stream<? extends T> a, final Stream<? extends T> b, final Stream<? extends T> c) {
         return Stream.concat(Stream.concat(a, b), c);
     }
 
     /**
      * 4 param variant of {@link Stream#concat(Stream, Stream)}
      */
     public static <T> Stream<T> concat(
-            Stream<? extends T> a, Stream<? extends T> b, Stream<? extends T> c, Stream<? extends T> d) {
+            final Stream<? extends T> a, final Stream<? extends T> b, final Stream<? extends T> c, final Stream<? extends T> d) {
 
         return Stream.concat(Stream.concat(a, b), Stream.concat(c, d));
     }
@@ -58,8 +61,8 @@ public static <T> Stream<T> concat(
      * 5 param variant of {@link Stream#concat(Stream, Stream)}
      */
     public static <T> Stream<T> concat(
-            Stream<? extends T> a, Stream<? extends T> b, Stream<? extends T> c, Stream<? extends T> d,
-            Stream<? extends T> e) {
+            final Stream<? extends T> a, final Stream<? extends T> b, final Stream<? extends T> c, final Stream<? extends T> d,
+            final Stream<? extends T> e) {
 
         return Stream.concat(
                 Stream.concat(Stream.concat(a, b), Stream.concat(c, d)),
@@ -70,8 +73,8 @@ public static <T> Stream<T> concat(
      * 6 param variant of {@link Stream#concat(Stream, Stream)}
      */
     public static <T> Stream<T> concat(
-            Stream<? extends T> a, Stream<? extends T> b, Stream<? extends T> c, Stream<? extends T> d,
-            Stream<? extends T> e, Stream<? extends T> f) {
+            final Stream<? extends T> a, final Stream<? extends T> b, final Stream<? extends T> c, final Stream<? extends T> d,
+            final Stream<? extends T> e, final Stream<? extends T> f) {
 
         return Stream.concat(
                 Stream.concat(Stream.concat(a, b), Stream.concat(c, d)),
@@ -82,8 +85,8 @@ public static <T> Stream<T> concat(
      * 7 param variant of {@link Stream#concat(Stream, Stream)}
      */
     public static <T> Stream<T> concat(
-            Stream<? extends T> a, Stream<? extends T> b, Stream<? extends T> c, Stream<? extends T> d,
-            Stream<? extends T> e, Stream<? extends T> f, Stream<? extends T> g) {
+            final Stream<? extends T> a, final Stream<? extends T> b, final Stream<? extends T> c, final Stream<? extends T> d,
+            final Stream<? extends T> e, final Stream<? extends T> f, final Stream<? extends T> g) {
 
         return Stream.concat(
                 Stream.concat(Stream.concat(a, b), Stream.concat(c, d)),
@@ -94,11 +97,24 @@ public static <T> Stream<T> concat(
      * 8 param variant of {@link Stream#concat(Stream, Stream)}
      */
     public static <T> Stream<T> concat(
-            Stream<? extends T> a, Stream<? extends T> b, Stream<? extends T> c, Stream<? extends T> d,
-            Stream<? extends T> e, Stream<? extends T> f, Stream<? extends T> g, Stream<? extends T> h) {
+            final Stream<? extends T> a, final Stream<? extends T> b, final Stream<? extends T> c, final Stream<? extends T> d,
+            final Stream<? extends T> e, final Stream<? extends T> f, final Stream<? extends T> g, final Stream<? extends T> h) {
 
         return Stream.concat(
                 Stream.concat(Stream.concat(a, b), Stream.concat(c, d)),
                 Stream.concat(Stream.concat(e, f), Stream.concat(g, h)));
     }
+
+    /**
+     * Conditionally sorts the stream based on presence of a comparator.
+     * @return null for null
+     */
+    public static <T> Stream<T> sortConditionally(
+        @Nullable final Stream<T> input,
+        @Nullable final Comparator<? super T> comparator) {
+        if(comparator==null) return input;
+        if(input==null) return null;
+        return input.sorted(comparator);
+    }
+
 }
diff --git a/...amodel/src/main/java/org/apache/causeway/core/metamodel/tabular/DataTableInteractive.java b/...amodel/src/main/java/org/apache/causeway/core/metamodel/tabular/DataTableInteractive.java
@@ -120,11 +120,12 @@ public Optional<Comparator<ManagedObject>> asComparator(final Can<? extends Data
 
     // -- SELECTION
 
-    Bindable<Boolean> getSelectAllToggle();
     void doProgrammaticToggle(Runnable runnable);
     Set<Integer> getSelectedRowIndexes();
     Observable<Can<DataRow>> getDataRowsSelected();
     void selectRangeOfRowsByIndex(IntStream range, boolean select);
+    void selectAllFiltered(boolean select);
+    void selectAll(boolean select);
 
     // -- EXPORTING
 

diff --git a/.../src/main/java/org/apache/causeway/core/metamodel/tabular/internal/DataTableInternal.java b/.../src/main/java/org/apache/causeway/core/metamodel/tabular/internal/DataTableInternal.java
@@ -39,6 +39,7 @@
 import org.apache.causeway.commons.internal.binding._Bindables;
 import org.apache.causeway.commons.internal.binding._Observables;
 import org.apache.causeway.commons.internal.binding._Observables.LazyObservable;
+import org.apache.causeway.commons.internal.collections._Streams;
 import org.apache.causeway.commons.internal.exceptions._Exceptions;
 import org.apache.causeway.core.metamodel.interactions.managed.ActionInteraction;
 import org.apache.causeway.core.metamodel.interactions.managed.CollectionInteraction;
@@ -108,7 +109,6 @@ public static DataTableInternal forAction(
     @Getter private final @NonNull LazyObservable<Can<DataRow>> dataRows;
     @Getter private final @NonNull LazyObservable<Can<DataRow>> dataRowsFilteredAndSorted;
     @Getter private final @NonNull LazyObservable<Can<DataRow>> dataRowsSelected;
-    @Getter private final _BindableAbstract<Boolean> selectAllToggle;
     @Getter private final _BindableAbstract<ColumnSort> columnSort;
 
     @Getter private final @NonNull LazyObservable<Can<DataColumn>> dataColumns;
@@ -150,10 +150,9 @@ private DataTableInternal(
                 .collect(Can.toCan()));
 
         this.dataRowsFilteredAndSorted = _Observables.lazy(()->
-            dataRows.getValue().stream()
-                .filter(adaptSearchPredicate())
-                .sorted(sortingComparator()
-                        .orElseGet(()->(a, b)->0)) // else don't sort (no-op comparator for streams)
+            _Streams.sortConditionally(
+                    dataRows.getValue().stream().filter(adaptSearchPredicate()),
+                    sortingComparator().orElse(null))
                 .collect(Can.toCan()));
 
         this.dataRowsSelected = _Observables.lazy(()->
@@ -162,15 +161,6 @@ private DataTableInternal(
                 .collect(Can.toCan()));
 
         this.selectionChanges = _Bindables.forValue(Boolean.FALSE);
-        this.selectAllToggle = _Bindables.forValue(Boolean.FALSE);
-        this.selectAllToggle.addListener((e,o,isAllOn)->{
-            //_Debug.onClearToggleAll(o, isAllOn, isClearToggleAllEvent.get());
-            if(isClearToggleAllEvent.get()) return;
-
-            doProgrammaticToggle(()->{
-                dataRows.getValue().forEach(dataRow->dataRow.getSelectToggle().setValue(isAllOn));
-            });
-        });
 
         this.searchArgument.addListener((e,o,n)->{
             dataRowsFilteredAndSorted.invalidate();
@@ -272,15 +262,6 @@ private Optional<Comparator<DataRow>> sortingComparator() {
     // -- TOGGLE ALL
 
     private final AtomicBoolean isProgrammaticToggle = new AtomicBoolean();
-    private final AtomicBoolean isClearToggleAllEvent = new AtomicBoolean();
-    public void clearToggleAll() {
-        try {
-            isClearToggleAllEvent.set(true);
-            selectAllToggle.setValue(Boolean.FALSE);
-        } finally {
-            isClearToggleAllEvent.set(false);
-        }
-    }
 
     @Override
     public void doProgrammaticToggle(final @NonNull Runnable runnable) {
@@ -297,8 +278,6 @@ public void doProgrammaticToggle(final @NonNull Runnable runnable) {
 
     void handleRowSelectToggle() {
         if(isProgrammaticToggle.get()) return;
-        // in any case, if we have a toggle state change, clear the toggle all bindable
-        clearToggleAll();
         invalidateSelectionThenNotifyListeners();
     }
 
@@ -308,6 +287,26 @@ private void invalidateSelectionThenNotifyListeners() {
         selectionChanges.setValue(!selectionChanges.getValue());
     }
 
+    @Override
+    public void selectAll(final boolean select) {
+        doProgrammaticToggle(()->{
+            dataRows.getValue()
+                .forEach(dataRow->{
+                    dataRow.getSelectToggle().setValue(select);
+                });
+        });
+    }
+
+    @Override
+    public void selectAllFiltered(final boolean select) {
+        doProgrammaticToggle(()->{
+            dataRowsFilteredAndSorted.getValue()
+                .forEach(dataRow->{
+                    dataRow.getSelectToggle().setValue(select);
+                });
+        });
+    }
+
     @Override
     public void selectRangeOfRowsByIndex(final IntStream range, final boolean select) {
         doProgrammaticToggle(()->{

diff --git a/...ract/src/test/java/org/apache/causeway/testdomain/interact/CollectionInteractionTest.java b/...ract/src/test/java/org/apache/causeway/testdomain/interact/CollectionInteractionTest.java
@@ -85,13 +85,12 @@ void multiSelect() {
                 expectedElements.get(3)));
 
         // toggle all on
-        tableTester.getDataTable().getSelectAllToggle().setValue(true);
+        tableTester.getDataTable().selectAll(true);
         tableTester.assertDataRowSelectionIsAll();
 
         // toggle all off
-        tableTester.getDataTable().getSelectAllToggle().setValue(false);
+        tableTester.getDataTable().selectAll(false);
         tableTester.assertDataRowSelectionIsEmpty();
-
     }
 
     @Test

diff --git a/viewers/wicket/ui/src/main/java/module-info.java b/viewers/wicket/ui/src/main/java/module-info.java
@@ -77,7 +77,6 @@
     exports org.apache.causeway.viewer.wicket.ui.components.layout.bs.col;
     exports org.apache.causeway.viewer.wicket.ui.components.widgets.bootstrap;
     exports org.apache.causeway.viewer.wicket.ui.components.actioninfo;
-    exports org.apache.causeway.viewer.wicket.ui.components.collection.bulk;
     exports org.apache.causeway.viewer.wicket.ui.pages.entity;
     exports org.apache.causeway.viewer.wicket.ui.components.scalars.bool;
     exports org.apache.causeway.viewer.wicket.ui.pages.accmngt.register;

diff --git a/...main/java/org/apache/causeway/viewer/wicket/ui/components/collection/CollectionPanel.java b/...main/java/org/apache/causeway/viewer/wicket/ui/components/collection/CollectionPanel.java
@@ -18,19 +18,15 @@
  */
 package org.apache.causeway.viewer.wicket.ui.components.collection;
 
-import java.util.Optional;
-
 import org.apache.wicket.feedback.ComponentFeedbackMessageFilter;
 
 import org.apache.causeway.commons.collections.Can;
 import org.apache.causeway.core.metamodel.tabular.DataTableInteractive;
 import org.apache.causeway.viewer.commons.model.components.UiComponentType;
 import org.apache.causeway.viewer.wicket.model.models.ActionModel;
 import org.apache.causeway.viewer.wicket.model.models.EntityCollectionModelParented;
-import org.apache.causeway.viewer.wicket.ui.components.collection.bulk.MultiselectToggleProvider;
 import org.apache.causeway.viewer.wicket.ui.components.collection.selector.CollectionPresentationSelectorPanel;
 import org.apache.causeway.viewer.wicket.ui.components.collection.selector.CollectionPresentationSelectorProvider;
-import org.apache.causeway.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ToggleboxColumn;
 import org.apache.causeway.viewer.wicket.ui.components.scalars.ScalarPanelAbstract;
 import org.apache.causeway.viewer.wicket.ui.panels.PanelAbstract;
 
@@ -46,8 +42,7 @@
 public class CollectionPanel
 extends PanelAbstract<DataTableInteractive, EntityCollectionModelParented>
 implements
-    CollectionPresentationSelectorProvider,
-    MultiselectToggleProvider {
+    CollectionPresentationSelectorProvider {
 
     private static final long serialVersionUID = 1L;
 
@@ -85,20 +80,4 @@ private void buildGui() {
         setOutputMarkupId(true);
     }
 
-    // -- MULTI SELECTION SUPPORT
-
-    private transient Optional<ToggleboxColumn> toggleboxColumn;
-
-    @Override
-    public ToggleboxColumn getToggleboxColumn() {
-        if(toggleboxColumn == null) {
-            var collModel = getModel();
-            var collMetaModel = collModel.getMetaModel();
-            toggleboxColumn =  collMetaModel.hasAssociatedActionsWithChoicesFromThisCollection()
-                    ? Optional.of(new ToggleboxColumn(collModel.getElementType(), collModel.delegate()))
-                    : Optional.empty();
-        }
-        return toggleboxColumn.orElse(null);
-    }
-
 }
diff --git a/...pache/causeway/viewer/wicket/ui/components/collection/bulk/MultiselectToggleProvider.java b/...pache/causeway/viewer/wicket/ui/components/collection/bulk/MultiselectToggleProvider.java
diff --git a/.../wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.css b/.../wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.css
@@ -119,7 +119,12 @@
 
 .collectionContentsAsAjaxTablePanel table.contents tbody .togglebox-column {
     width: 2%;
+    text-align: center;
+}
+.collectionContentsAsAjaxTablePanel table.contents thead .togglebox-column .dropdown-item li {
+	opacity:1;
 }
+
 .collectionContentsAsAjaxTablePanel table.contents thead .title-column,
 .collectionContentsAsAjaxTablePanel table.contents tbody .title-column {
     text-align: center;

diff --git a/...wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.java b/...wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.java
@@ -21,7 +21,6 @@
 import java.util.List;
 import java.util.Optional;
 
-import org.apache.wicket.Component;
 import org.apache.wicket.extensions.ajax.markup.html.repeater.data.table.AjaxFallbackDefaultDataTable;
 import org.apache.wicket.markup.head.CssHeaderItem;
 import org.apache.wicket.markup.head.IHeaderResponse;
@@ -38,7 +37,7 @@
 import org.apache.causeway.core.metamodel.tabular.DataTableInteractive;
 import org.apache.causeway.viewer.wicket.model.models.EntityCollectionModel;
 import org.apache.causeway.viewer.wicket.model.models.EntityCollectionModel.Variant;
-import org.apache.causeway.viewer.wicket.ui.components.collection.bulk.MultiselectToggleProvider;
+import org.apache.causeway.viewer.wicket.model.models.EntityCollectionModelParented;
 import org.apache.causeway.viewer.wicket.ui.components.collection.count.CollectionCountProvider;
 import org.apache.causeway.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ActionColumn;
 import org.apache.causeway.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ColumnAbbreviationOptions;
@@ -49,6 +48,7 @@
 import org.apache.causeway.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.TitleColumn;
 import org.apache.causeway.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ToggleboxColumn;
 import org.apache.causeway.viewer.wicket.ui.components.table.CausewayAjaxDataTable;
+import org.apache.causeway.viewer.wicket.ui.components.table.DataTableWithPagesAndFilter;
 import org.apache.causeway.viewer.wicket.ui.components.table.filter.FilterToolbar;
 import org.apache.causeway.viewer.wicket.ui.panels.PanelAbstract;
 import org.apache.causeway.viewer.wicket.ui.util.Wkt;
@@ -121,10 +121,6 @@ private void buildGui() {
                 getWicketViewerSettings(),
                 columns);
 
-        // prepend toggle-box column (left most), if enabled
-        getToggleboxColumn()
-            .ifPresent(toggleboxColumn->columns.add(0, toggleboxColumn));
-
         // last append action column
         //TODO[CAUSEWAY-3815] disabled until action column rendering is fleshed out
         addActionsColumnIfRequired(elementType, columns);
@@ -134,21 +130,21 @@ private void buildGui() {
                 ID_TABLE, columns, dataProvider, collectionModel.getPageSize());
         addOrReplace(dataTable);
 
+        // prepend toggle-box column (left most), if enabled
+        createToggleboxColumn(dataTable)
+            .ifPresent(toggleboxColumn->columns.add(0, toggleboxColumn));
+
         addFilterToolbar(dataTable);
     }
 
-    private Optional<ToggleboxColumn> getToggleboxColumn() {
-        return getMultiselectToggleProvider()
-                .map(MultiselectToggleProvider::getToggleboxColumn);
-    }
+    // -- MULTI SELECTION SUPPORT
 
-    private Optional<MultiselectToggleProvider> getMultiselectToggleProvider() {
-        Component component = this;
-        while(component != null) {
-            if(component instanceof MultiselectToggleProvider) {
-                return Optional.of((MultiselectToggleProvider) component);
-            }
-            component = component.getParent();
+    private Optional<ToggleboxColumn> createToggleboxColumn(final DataTableWithPagesAndFilter<?, ?> dataTable) {
+        if(entityCollectionModel() instanceof EntityCollectionModelParented collModel) {
+            var collMetaModel = collModel.getMetaModel();
+            return collMetaModel.hasAssociatedActionsWithChoicesFromThisCollection()
+                    ? Optional.of(new ToggleboxColumn(collModel.getElementType(), dataTable))
+                    : Optional.empty();
         }
         return Optional.empty();
     }