Add new Column Config option: isHidden (to be included in grid but not displayed by default)

[paugnu]

Hi,

I think it might be interesting to add a new Column Config option: isHidden (note that this is different from 'Exclude').

The idea is to have the column available to be displayed, but not visible by default.

I'll try to implement this myself, I have a lot to learn though :)

[Vinai]

Thanks for your suggestion, makes sense. This is interesting because the column property from the grid XML influences the initial settings for the JS Display dropdowns. If you want to continue, let me know please if you have specific questions regarding the implementation. Should you run out of time or motivation though, no problem :) I or someone else can pick up the issue.

[paugnu]

I think I can do it myself :) (I've already added and retrieved the isHidden column definition). But in terms of frontend, the logic seems to be trickier:

• At this moment we have a session variable where we only store the columns that are 'hidden'
• We cannot extend directly that feature because it might incur in some incoherence: we need to be consistent when saving each column visibility status and correctly store what the user has selected as visible/not visible

I'll work on this :)

[Vinai]

I would like to write up a few bullet points (in this thread) how I would go about adding a new column option when I have time this afternoon.

Regarding the frontend state in JS, currently the array with hidden columns is initialized as an empty array. I guess adding the column keys where isHidden="true" there when the array is initialized (that is, when it is not already set in the session storage) would take care of it.

[Vinai]

@paugnu I'm writing this down even though you probably already figured all or most of it out on your own. I'm doing this so I have a place where it is in writing, to point myself and others to in future.

That said, here are the steps I would take to add a new column property:

1. The first thing I'd do is I'd add the property to a test in the class \Hyva\Admin\Test\Unit\Model\Config\GridXmlToArrayConverterTest (maybe extend the methods getColumnsXml and getColumnsExpected.
2. Once that causes the test to fail, I would then add the required code to make the test pass to \Hyva\Admin\Model\Config\GridXmlToArrayConverter.
3. The next step is to add the new attribute to Hyva/Admin/etc/hyva-grid.xsd. Now the new attribute can be specified and will be included in the column configuration.
4. The next step is to add the property to the \Hyva\Admin\ViewModel\HyvaGrid\ColumnDefinitionInterface and \Hyva\Admin\ViewModel\HyvaGrid\ColumnDefinition. This the serialization via toArray also needs to include the new property for proper column definition merging. There are a number of tests that probably will need to be adjusted for the new column.
5. Now the new column property can be used in the template, e.g. in the getHiddenCols method where the array is initialized when there is no value in the session storage.

[Vinai]

While looking over the code to write the above steps, I noticed the ColumnDefinition::$isVisible property already exists. It indicates if a column should be rendered or not, based on whether it is excluded or not included in the grid columns definition.

This in unfortunate, because adding a property with the name isHidden will lead to a lot of confusion. It would seem that the properties isVisible and isHidden are related and always should be the inverse of each other. In fact both properties don't have any relation.

With the benefit of hindsight, the isVisible property should have been called isRendered or something along those lines, but now it is already how it is, and breaking changes are not an option for me. For that reason, isHidden will have to get a more specific name. Naming things is hard :)

So, here are the goals of the name:

• descriptive of it's purpose
• clearly distinct from isVisible

First ideas:

• isInitiallyHidden This would indicate that the value might change, implied by the word initially
• isHiddenByJs This would indicate that it is distinct from the isVisible property because it includes the reference to JavaScript. As a developer, this might lead me to the conclusion that isVisible then does not involve JavaScript as it is lacking the ByJs in the name.

Of those two I like isInitiallyHidden better, because it's less clunky than the ByJs one, but all in all I don't think either is very good. I would love it if you or someone else come up with a better name for the new property.

[paugnu]

I would like to write up a few bullet points (in this thread) how I would go about adding a new column option when I have time this afternoon.

OK, I see you've already done it! while I was answering :). I agree on your concerns about 'isVisible'. Actually, before opening the issue I did some checks on his to be sure this Column Definition wasn't what I was actually defining as 'isHidden'.

From my point of view, as a dev, 'isInitiallyHidden' defines clearly what we I would like to achieve.

Regarding the frontend state in JS, currently the array with hidden columns is initialized as an empty array. I guess adding the column keys where isHidden="true" there when the array is initialized (that is, when it is not already set in the session storage) would take care of it.

It makes sense. I'll implement it and check of the possible cases to be sure it works fine :)

[Vinai]

Released in 1.0.8