You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by GitBox <gi...@apache.org> on 2021/12/30 06:56:19 UTC

[GitHub] [wicket] RLStokes opened a new pull request #491: WICKET-6941 Add visible attribute to AbstractColumn to hide/show columns in DataTable/DataGridView

RLStokes opened a new pull request #491:
URL: https://github.com/apache/wicket/pull/491


   Not sure about the change to the interface ICellPopulator whether it is allowed in a minor version update.  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [wicket] reiern70 commented on a change in pull request #491: WICKET-6941 Add visible attribute to AbstractColumn to hide/show columns in DataTable/DataGridView

Posted by GitBox <gi...@apache.org>.
reiern70 commented on a change in pull request #491:
URL: https://github.com/apache/wicket/pull/491#discussion_r776660926



##########
File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/repeater/data/table/HeadersToolbar.java
##########
@@ -71,7 +71,10 @@
 
 				for (IColumn<T, S> column : table.getColumns())
 				{
-					columnsModels.add(Model.of(column));
+					if (column.isVisible())

Review comment:
       I'm not sure this is the only involved toolbar that needs to be fixed... Did you run
   
   `mvn clean install`
   
   ?

##########
File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/repeater/data/grid/ICellPopulator.java
##########
@@ -71,4 +71,8 @@
 	 */
 	void populateItem(final Item<ICellPopulator<T>> cellItem, final String componentId,
 		final IModel<T> rowModel);
+		
+	boolean isVisible();

Review comment:
       I would prefer this to be be 
   
   ```
   default boolean isVisible() {
       return true;
   }
   
   ```
   
   and no setter.

##########
File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/repeater/data/grid/PropertyPopulator.java
##########
@@ -74,4 +75,16 @@ public void populateItem(final Item<ICellPopulator<T>> cellItem, final String co
 	{
 		cellItem.add(new Label(componentId, new PropertyModel<>(rowModel, property)));
 	}
+
+	@Override
+	public boolean isVisible()

Review comment:
       See my comment above.

##########
File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/repeater/data/table/AbstractColumn.java
##########
@@ -37,6 +37,8 @@
 
 	private final S sortProperty;
 
+	private boolean visible = true;	

Review comment:
       See comment about read only

##########
File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/repeater/data/grid/ICellPopulator.java
##########
@@ -71,4 +71,8 @@
 	 */
 	void populateItem(final Item<ICellPopulator<T>> cellItem, final String componentId,
 		final IModel<T> rowModel);
+		
+	boolean isVisible();
+	
+	void setVisible(boolean visible);

Review comment:
       As said I would prefer this to be only read-only




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [wicket] reiern70 commented on a change in pull request #491: WICKET-6941 Add visible attribute to AbstractColumn to hide/show columns in DataTable/DataGridView

Posted by GitBox <gi...@apache.org>.
reiern70 commented on a change in pull request #491:
URL: https://github.com/apache/wicket/pull/491#discussion_r776662824



##########
File path: wicket-extensions/src/main/java/org/apache/wicket/extensions/markup/html/repeater/data/grid/ICellPopulator.java
##########
@@ -71,4 +71,8 @@
 	 */
 	void populateItem(final Item<ICellPopulator<T>> cellItem, final String componentId,
 		final IModel<T> rowModel);
+		
+	boolean isVisible();

Review comment:
       Also JavaDoc.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [wicket] reiern70 commented on pull request #491: WICKET-6941 Add visible attribute to AbstractColumn to hide/show columns in DataTable/DataGridView

Posted by GitBox <gi...@apache.org>.
reiern70 commented on pull request #491:
URL: https://github.com/apache/wicket/pull/491#issuecomment-1002964348


   And thanks for your work :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org