Wow. This module is pure, lean and mean genious.
Would you accept a patch that uses CCK's registry of 'build modes' (content_build_modes() ), when available, to enrich the current 'display on teaser / full node' option ? That would probably mean turning the option into a single list of checkboxes.
I'll probably be needing this for the project I'm currently working on, so we'll be writing this anyway.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | views_attach_fix_modes.patch | 2.66 KB | yched |
| #10 | views_attach_2.patch | 2.46 KB | George2 |
| #9 | views_attach.patch | 5.67 KB | yched |
| #6 | views_attach.patch | 5.67 KB | yched |
| #4 | views_attach_build_modes.png | 1.48 KB | yched |
Comments
Comment #1
Crell commentedThat sounds like an awesome idea. :-) I'm not sure how feasible an upgrade path would be for a View. We'd have to check with merlin. But yeah, highly cool. Patch welcome.
Comment #2
aleksey.tk commentedHi! I created a patch (with help from Yves) for implementing 'build_nodes' registry in views_attach display settings. Work is not done fully, but it is a good starting point (for UI polishing, changes, etc ). Please have a look at it!
P.S. Yves got his version of patch (much better then mine), but it is not completed (an issues with backward compatibility if cck is not enabled on the site), so we decided to give you my patch for reviewing.
Comment #3
Crell commentedThis looks good visually. No time at the moment to test it. Should we be using this patch, or waiting for another from Yves?
Comment #4
yched commentedPatch in #2 stays inline with the current UI : you get one separate setting for each build mode (see screenshot). This makes it easier to ensure backwards compatibility with views defined with the current views_attach 1.0 without needing them to be manually edited. On the downside, this makes the UI a little tedious.
I then tried to expand on this initial patch to move to a single, select-based setting, but so far haven't been able to have it preserve backwards compatibility without painful amounts of legacy code. I might give it another try shortly, but am not expecting miracles :-)
And the final CCK D6 release showed that there is no good way to automatically update views definitions...
Comment #5
Crell commentedThis is a young enough module that I'm OK with a BC break if necessary. It's not a huge deal for people to update their views if we warn them about it.
So the patch in #2 just adds another option param for each build mode, da? I thought we were going to move to something like a checkboxes list, although I can see the simplicity in this approach. Let's get some additional input before committing to one way or another. If the best solution going forward involves a small BC break, I'm fine with that.
Comment #6
yched commentedAlrighty :-) Attached patch uses the logic in cyberpunk's patch, but wraps it in a single (select based) setting.
AAMOF, no BC shouldn't be too much of an issue, since most existing node_content displays probably use 'display on full node' (because of #362670: is this really usable outside node main page ?), which the default setting they will get with this patch.
Until we know more about #362670: is this really usable outside node main page ?, the main interest of this patch is to save CPU on non-full / non-teaser build modes (currently, they are treated like 'full')
Comment #7
Crell commentedWhy select based instead of checkboxes?
Comment #8
yched commentedDoh, did I say select ? Sorry, I meant checkboxes. Late, tired :-)
Comment #9
yched commentedNew patch fixes a variable name clash that caused the node_content views 'fields' to disappear intermittently from the 'Manage fields' CCK screen
Comment #10
George2 commentedgreat module, but confusing!
after applying this patch, and then go to create a new view, i get the error:
parse error, unexpected T_ISSET, expecting '(' in sites\all\modules\views_attach\views_attach.module on line 78
line 78: if isset($node->build_mode) {
oopsie ;)
and seeing "full, full, 0, 0, 0" after clicking ok after checking out what build modes is, is kinda scary. is it meant to do that?
and crell already knows, but i find the label "embed this display in the following node types" REALLY confusing. i.e. i wanted to embed fields of b into a. so, i chose a here because i wanted to embed this display in a when after MUCH trial and error, i find i should be choosing b. i don't understand why, but now it works. however, i am using a nid in the arg, and nodereference relationship setup - is that confusing things?
anyway, rerolled (i think!) - my first one, so please forgive stupid mistakes :(
Comment #11
Crell commentedThe patch in #10 is missing some code, apparently.
This patch was also broken by #362670: is this really usable outside node main page ?, which I committed first. So I went through and merged it in. I also corrected the "full full 0 0 0" issue, which is cuased by content_build_modes() returning a very unpredictable array. So I compensated.
Committed to dev. Thanks everyone! I will hopefully be rolling a new stable soon.
Comment #12
yched commentedReopening. Looks like I fubar'd along the way of testing this. The current code randomly returns inconsistent data in views_attach_get_node_views(). Really sorry about this. Too busy days, I guess.
Attached patch fixes this,
+ removes the comment about CCK, which IMO is unfair ;-). content_build_modes() internals are butt-ugly but it has its reasons, and its ugliness is hidden in views_attach_build_modes(). In options_summary(), we loop through the array just because we need to display translated, human readable values for the machine names we store in the view options.
+ adds a break; in views_attach_nodeapi() mainly so that I quit wondering about the indentation gap at the end of the function every time I go over it.
+ contains the quickfix in http://drupal.org/node/362670#comment-1263134 because it makes no sense to artificially leave it out.
Comment #13
yched commentedComment #14
Crell commentedOopsies. Yeah, I had a number of bugs that this fixed. Committed, thanks. I will wait a bit before rolling a new stable to see if anything else crops up, but please use the dev for now. It is working much better than 1.1 is for me right now.