Support for Grouped Views
| Project: | Views |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Crell |
| Status: | closed |
After some discussion with merlin earlier today, here's a first crack at support for grouped views in Views 2. It works like this:
If a view is using a row plugin that supports fields, the views_plugin_style class will auto-add a form fragment to select the grouping field. Child classes must call parent::options_form(). On render, then, the renderer will group the incoming data by the field in question. How to display the label of each group (the grouping field) is left up to each theming routine. For unformatted, list, and grid views, I've made it an h3 tag to parallel theme('item_list'). For table views, it's the caption of the table. Currently all four included style plugins are supported.
Note that I had to change the way table views are themed slightly. Rather than having their rows pulled out of the view in the preprocess function, they are passed in just like any other plugin. That's necessary so we can do the grouping/filtering back up in render(), so that each table template is still dealing with only a single table/group.
There are two things that remain to be done, but I wanted to get this posted:
1) Currently the grouping field/title of each group is the raw version of the field, not the formatted one. I do not like that. I am not, however, sure how to render just the one field from within the render() method. Earl?
2) I want to add support for excluding the grouped field from the actual displayed results. That is, if you group by the node type field, then node type is not listed in each list/table. While that can be done with a targeted template file for that specific field/view, that's icky and doesn't really work for tables. It should be handled via a checkbox option to make it easy.
Any help with those two would be much appreciated.
This is my first crack at Views 2 code since November or so; it's SO much easier to work with than Views 1's API. :-)
(The patch is rolled against beta2, since that's what I already had running, but it should apply to dev as well, I think.)
| Attachment | Size |
|---|---|
| grouped_views.patch | 8.18 KB |
#1
I'm not here for long so this is not a full review.
+ function options(&$options) {+ parent::options($options);
+ }
That's not necessary. =)
#2
Hm. That is indeed true. :-) I think I included that because the method was already there and that class has extra options that need to be added to it anyway according to the @todo in options_form(). (That's my story and I'm sticking to it!)
#3
Fair enough; it's a really minor minor point.
The more interesting point is: At some point this feature may be required to expand to allowing nested grouping. i.e, group on 1 field and then again on another field. I'm a little concerned that the approach taken will basically not allow that expansion. And while I don't think that feature needs to be in, I would prefer that this feature be implemented in such a way that adding more grouping levels doesn't necessitate rewriting the whole feature.
Thoughts?
#4
I was afraid someone would talk about that... :-) I actually considered multi-level nesting, but I wasn't sure how it would actually work. I am open to it if you can suggest a method for implementing it. That would still require figuring out both of the issues above.
The main challenge to multi-level nesting is that to make it unlimited, we would probably need a recursive call. Those are messy. We'd also need some way in the form to specify which fields to group on, with an unlimited variable number. Enabling multi-level grouping would, I think, require implementing it now (because by the time we added something that could handle it later, we may as well just finish it).
#5
Oh, to answer your actual questions (sorry, I'm slow apparently):
$view->field[$id]['handler']->theme($row) <-- that properly renders a field, running it through the appropriate theme function.
It would be fairly trivial to do something like: $handler->do_not_render = TRUE; and then in template_preprocess_views_view_fields check for that flag to be empty() at line 144 of theme.inc. That'll make it possible to pull fields out of the fields.
I should think you could do multi-level nesting this way: Set up all fields as checkboxes. Check the fields you want grouped. The order that they appear in is the order they will group in. The recursion is probably ok, but I would beware; I don't think it's as simple as just running the style theme repeatedly, because lists should actually show up within lists:
group Ao group B
x item
x item
o group B2
x item
x item
group A2
...
Group A is a caption, but then group B is inside the <li>. So the existing style handlers have to become group aware. This is ok, I think, as we can set a flag or something on the handler definition, and handlers that don't have it are not group aware and should not run the grouping logic.
#6
So, is this ready for review of is Crell planning some expansions?
#7
I'm going to try and implement merlin's comments in #5 (I just got back from a short vacation), but I can't guarantee a timeframe so feel free to review now if you'd like.
#8
OK, here we are again. Attached version does pretty much everything we've discussed so far except for recursive grouping. To wit:
- You can specify a grouping field on the style plugin config form.
- The grouping field will be rendered, and the rendered version will be what is grouped on.
- You can specify that a field should not be rendered, just pulled for its raw data. That's per-field and is not related to grouping at all, except that the two likely will be used together a lot. You can easily have non-displaying fields for other purposes, though.
- Tables are annoying, but I got them behaving the same as everything else.
Also, this patch is now against HEAD/6.x-2.x-dev.
I'm game to save recursive grouping for later.
#9
So I finally had some time to come look at this patch and I noticed something: You forgot to attach the patch. I feel bad that it took me this long to notice that. :/
#10
Oh for the love of pete...
#11
Ok, I've played a little bit with this patch and I can't apply this as is.
+ function options_form(&$form, &$form_state) {+ // Only fields-based views can handle grouping.
+ if ($this->uses_fields()) {
There needs to be an easy way for a style to say it does not want grouping, and not just because it doesn't use fields. Add
&& !empty($this->definition['uses grouping'])is probably sufficient here (and a comment to doc this in the doc.php since none of the core styles will use it).+ if ($this->options['grouping']) {+ foreach ($this->view->result as $row) {
+ // Group on the rendered version of the field, not the raw. That way,
+ // we can control any special formatting of the grouping field through
+ // the admin or theme layer or anywhere else we'd like.
+ $grouping = $this->view->field[$this->options['grouping']]['handler']->theme($row);
+ if ($this->options['grouping_exclude']) {
+ //$this->view->field[$this->options['grouping']]['handler']->options['exclude'] = TRUE;
+ }
+ $sets[$grouping][] = $row;
+ }
A lack of empty() in that if causes existing installations to throw notices left and right; also, this code is basically duplicated in at least one place which suggests it should be abstracted into a base method and called to reduce the workload for styles that have complex render() methods.
However, what got me to stop was the commented out line on the exclude. At this point I wasn't quite sure what your intention was.
- $result = $view->result;+ $result = $vars['rows'];
+ // We're going to rebuild this variable below, but need to empty it first.
+ $vars['rows'] = array();
+
I couldn't figure out the intent here; just deleting it doesn't work, as it destroys something later you didn't touch. Which confuses me.
Finally, while I was working with this it occurred to me: For this grouping, do we want to limit grouping to fields that are 'sortable' and then have them automatically sort the query? Because grouping without sorting seems like a bad idea.
#12
In leiu of a review, I post this screenshot from Textmate. This is a command to list all todo lines in your file. The relevant point is how it groups by TODO priority (I use TODOH, TODO, TODOL for high/medium/low priority). Note the hyperlinked summary counts at the top, the use of color in summary and in header lines, and the repeating of table headers in each group. If our grouping plugin had options to do all this, I'd be might impressed.
#13
I think the 'attach' feature of Views2 is how the Summary stuff gets done. It would take some work to get it to hyperlink and color like the screenshot above. Anyway, I think this screenshot is a great goal for this feature.
#14
Moshe: That's very pretty, but most of what it's doing is in the category of theming, IMO.
#15
Attached is a re-rolled patch with a couple of outdated hunks fixed. The 'group by:' select box was also dependent on the field having a label, previously. The new version uses $field_label ($field_id) so that the dropdown never shows empty rows. I've tested it and am using it to good effect -- there are obviously all sorts of things I'd love to see added, but this is a solid start and it works smoothly.
#16
Doesn't include Merlin's requests from #11, by the way. The above patch is just the stuff I did in order to get it working with the current HEAD version of views.
#17
Thanks, eaton!
Attached patch addresses the comments from #11. Cleaned up various notices, moved the grouping loop to a separate utility method, and better documented the weirdness in the preprocess function. Also added a "uses grouping" key for styles to opt out of grouping.
That should take care of everything except multi-level grouping, which I don't think I want to try and tackle right now as it would be highly messy.
#18
Could we enrich the CSS a bit so that we titles and rows are have grouping specific classes?
#19
I mean, enrich the markup so that a CSS wiz can do nice stuff out of the box.
#20
The view will (I think?) have various classes on it already. (At least it did in Views 1.) Just style the h3 tag inside that view div, or the table caption inside that view div. Add salt to taste. :-)
#21
i propose *grouping specific classes* on titles and rows .. i only read through the patch so i might be off base but this seems useful to me and will help achieve the look i suggested in a screenshot earlier.
#22
Right, some kind of counter, maybe, which could be stored on the style handler and put into the variables in the preprocessor perhaps?
#23
Hm. I think the cleanest way to do that would be to always wrap a set/group in another div with the counter in it; but for non-grouped views I don't know how useful that would be, and it adds another div. That would probably have to be kept in the loop counter in the render() method and passed through as another theme() parameter. merlin, does that sound sane?
#24
What if we took the text for the group, converted into something css safe (though, hm. We still don't have really good text to css safe so maybe this is a bad idea) and put it in the preprocess as a group identifier; if it exists the highest level CSS could add it like so: views-group-NAME
But a counter could work the same way. I wouldn't want another div but there should be at least one place we could stick it in for every template.
#25
Well the label is the rendered form of the field, not the raw form, so it could very easily be a sentence, or even an image or file attachment. Either way it will have HTML in it. Not really suitable for a CSS class.
#26
Committed. Moshe, you might want to open a feature request to add CSS ids, but I'm not sure it'll make it; if they're just numbered that might be kind of rough. But I'm not sure yet.
#27
yay!
#28
Automatically closed -- issue fixed for two weeks with no activity.