-
Notifications
You must be signed in to change notification settings - Fork 394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WICKET-6941 Add visible attribute to AbstractColumn to hide/show columns in DataTable/DataGridView #491
base: wicket-9.x
Are you sure you want to change the base?
Conversation
…mns in DataTable/DataGridView.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand the use case for this. A table which dynamically can have more or less columns? I would not mind this change is code is made read only with a default method in interface.
@@ -71,4 +71,8 @@ | |||
*/ | |||
void populateItem(final Item<ICellPopulator<T>> cellItem, final String componentId, | |||
final IModel<T> rowModel); | |||
|
|||
boolean isVisible(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this to be be
default boolean isVisible() {
return true;
}
and no setter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also JavaDoc.
|
||
boolean isVisible(); | ||
|
||
void setVisible(boolean visible); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As said I would prefer this to be only read-only
@@ -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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above.
@@ -37,6 +37,8 @@ | |||
|
|||
private final S sortProperty; | |||
|
|||
private boolean visible = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment about read only
@@ -71,7 +71,10 @@ | |||
|
|||
for (IColumn<T, S> column : table.getColumns()) | |||
{ | |||
columnsModels.add(Model.of(column)); | |||
if (column.isVisible()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is the only involved toolbar that needs to be fixed... Did you run
mvn clean install
?
And thanks for your work :-) |
Close/reopen to re-trigger the build checks. |
Not sure about the change to the interface ICellPopulator whether it is allowed in a minor version update.