Add filter groups

[joemsak]

This PR adds a filter_group attribute to filters, which is set via the options hash

Its purpose is to allow filters to be grouped together, to enhance theming ability, as the form partial will add a wrapping div based on those filters.

When I added the view logic to the partial and the test partial, no tests failed, so I'm not sure where I should have added a test for this new HTML logic. Would love feedback on that, and I can add the tests as needed.

[bogdan]

There are many problems with that: at first - I am not sure it will fit everyones needs. In my project for example, we would need to attach additional options to the filter group and also allow nested groups too.

At second, this is a breaking change because you changed the DOM inside datagrid form. It may result in broken layout when people upgrade.

At third, this change is implementable on you end easily using rake datagrid:copy_partials and modifying the form.html.erb in the same way.

[joemsak]

On your third point, this change is implementable by me because I modified your source code and I'm running on my own branch of the code, hence why I suggested the feature via a PR.

[bogdan]

It is not necessary to modify anything except form.html.erb to implement your change. You can do that on your end by running the rake command I've specified.

[joemsak]

@bogdan I don't think so, I need them to be grouped in a wrapping div, so that they will stay together in groups and I can arrange the groups. I did override the views and I could think of no way to pull it off dynamically without having an option in the DSL itself to produce the grouped filters. What would you suggest instead?

I've thought some more on your other points:

On your second point, I don't see it as a breaking change because anyone who did override the form partial would not notice any difference, and anyone using the built in one should also not notice anything because everything would be in one blank string group, and wrapped in an unstyled div, still in the order they were specified in their ThingsGrid class

On your first point, I think you shut down and closed my PR too quickly before it could have a chance to be discussed by the community of your users (of which I am one, who took the time to care and contribute, only to be shut down extremely fast ... doesn't feel great)

[bogdan]

You don't need DSL: all filter options are available via #options method so you would just need the following in your own copy of _form.htmlerb:

grid.filters.group_by{|f| f.options[:filter_group]}.each ...

Big deal?

On the first point: Closing PR doesn't forbid anyone to discuss. I am not leaving you without my responses anyway.

On the second point: the unstyled div has "display: block". You don't know the UI used by all datagrid users and you have no idea what kind of affect it will have for all possible stylesheets applied to default form markup. I only accept structural changes to built-in partial if they really make sense.

[joemsak]

ah okay, I didn't think of using the options in that way, that is certainly an avenue I can take, thanks

Point taken on the div being a display block element, though the filters themselves already appear in divs, so I couldn't say how much effect it would generally have; using the correct release tag format to indicate a warning for backwards compatibility for upgrading is usually enough, though.

And honestly I doubt a closed PR would get as much visibility as an open one for discussion, but it's your repo

I will revert back to using your gem source and the options method as you suggested, thanks