| Project: | CCK Blocks |
| Version: | 6.x-1.x-dev |
| Component: | User interface |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Steven Jones |
| Status: | closed (fixed) |
Issue Summary
If you have a lot of CCK fields, your blocks configuration page can easily get cluttered. Even worse, you can easily mix up one field with another on this page, and publish fields as a block that were never meant to be used as such.
A solution would be to have a separate configuration option for each CCK field that defines whether this field should be available as a block. There can be a checkbox within the individual field configuration page, plus a configuration page with a list of fields and checkboxes (maybe as a sub-tab in the blocks config page, so you have it easily available).
The problem would not exist (or be less severe) if the inactive blocks in the blocks config page were grouped by module (they would have to be re-sorted every time you submit).
Comments
#1
Marking that as critical, as I do agree completely, that a huge number of cck fields can turn the list of blocks into a mess.
Maybe we can have a look at the nodeblock module (http://drupal.org/project/nodeblock). I guess, they do, what you suggested, not with cck fields but with content types.
#2
I think it is important that you have the configuration page easily available from the blocks admin page, so you don't need to scratch your head where the CCK blocks have gone.
Where should the information be stored? In content_node_field.global_settings, or somewhere in content_node_field_instance? I think it would make sense to do both, and allow to override the setting for field instances.
#3
nodeblock: You can't really compare that, I think. If nodeblock would not have a checkbox per content type, you would end up with tens of thousands of blocks, as many as you have nodes. As it is now, I only have as many block nodes as I need, which is around 2 or 3 of them.
#4
Hm, let me see if I understood the difference. I guess:
- content_node_field.global_settings are the global settings of a field, regardless of the content type, the field is assigned to
- content_node_field_instance are the instance specific settings for a field, attached to a node
In my opinion, we should avoid storing (and presenting) this option at more than one configuration page. I guess, one does not have that many node types, that it's too much work to enable/disable the "show as block" feature for every instance of a field, you want to have in a block. I guess, there are not more than 10-20 blocks on one paga and not every block will display a cck field.
So my recommendation is to provide this option for field instances only. Maybe
admin/content/node-type/*/displayis the right place to add an checkbox "display as block". Alternatively, a checkbox in the fields configuration page (at the bottom, in a "cck blocks" area) is possible.#5
I have a use case for storing things in content_node_field.global_settings.
I made a CCK field (flexifield with imagefield + radiobutton options) that allows to define background images for nodes of different content types. There is a "content_bg" region where the blocks work as page backgrounds, and the CCK block for this flexifield is one of them.
To make a new content type that can do background images, I simply give it this flexifield. And for pages that are not nodes, I have a nodeblock that does the same job. Advanced shit.
It would be quite a few steps more work if I had to configure block visibility separately for every content type. The field is made for being a block, no matter which content type it is used in.
And that aside, the module provides one block per field, not per field instance. That's how it works.
I think it would make sense to provide both options, but have a central place to manage it all.
#6
Obviously ;-)
So, in the first step, I would recomend to provide an option for activating / deactivating the block per field, as it seems, that a field should have the same meaning in every content type it is used with.
I guess it's not too much work, if you need the same functionallity without a block, to copy the field and then to have to fields, say, "field_background_block" and "field_background_no_block".
Unfortunately, I'm out of my office for the next three weeks, but i'm looking forward to the discussion, that is hopefully going on here without me. In my opinion, his is an important topic!
#7
Please have a look at tomorrows -dev release. Do you have enough experience in cck to see, where to make the changes to solve this issue?
#8
Subscribing.
#9
subscribe
#10
This would be helpful. Subscribing.
#11
We'll have a patch for this by the end of the day! Hurrah!
#12
To avoid the global vs. instance debate, I've implemented both options, with global and instance specific preferences. This is my first patch, please be nice! critique welcome.
#13
Hey steve,
that looks awsome! I didn't test it, yet, but the code looks like a good job.
Just one question about the update: do you change the state of each block to "enabled" by default? Is this only applied for users, who already have installed cck_blocks to preserve the current behaviour of the system?
#14
@Earl Grey - Yes, so that existing user's weren't like: 'Where have all my blocks gone!' But existing users will not have to contend with hundreds of blocks.
#15
The patch seems to work perfectly!
There's just one addition: I assume, that most users don't want to use most of the fields as block. Therefor, I tink it would be better to disable the block visibility of all fields, that are currently not enabled as block.
Do you see any chance to get this done via the db update?
#16
Don't forget there can be different settings in different themes!
I only had a short glance at the patch (sorry)..
Which forms do you provide to control the CCK blocks settings? Is this part of the CCK content type and field management, or do you have one central form to rule them all? I think it would make sense to have both.
#17
Thank you for the hint about the themes. It should work something liike that: select all activated cck_blocks from the blocks table, ignoring the theme, just looking at their status. After that, we can deactivate all blocks that are not in that list. I tried that with some db queries, but I'm not sure, how to compare the fields with the blocks. None of my ideas worked.
At the moment, there is only the UI at the fields configuration page, what is much better as what we have had before the patch.
I'd like to commit that patch, as soon as we have solved the update 'problem'. Improving the UI is still possible in the future.
#18
It could still cause unpleasant suprises for existing installations, if some blocks are disabled upon module update. Think of other modules (Panels?) that make use of blocks, or projects where the client likes to play with the blocks page.
So what about:
- For new installations of this module, no fields are enabled as blocks.
- For updated installations, all fields are enabled as blocks.
- After the update, the admin will see a message saying he should visit the CCK blocks page, and decide which of the fields should be available as blocks. Similar to "Content permissions need to be rebuilt.". This message will go away the first time the management form is succesfully saved.
- The management form (fields available as blocks) will find out which of the blocks are used in any of the available themes.
#19
@Earl Grey - We thought it was a bad idea to change the behaviour of the CCK Blocks module in such a fundamental way for people upgrading, I think it makes more sense to keep the status quo for those people, but try to educate them that they can turn off the blocks exposed if they want.
We don't actually need to care what blocks are used in any theme, because it's the same set of blocks offered to each theme. I think that if we need to add additional UI then we can do this in a follow up patch, it should be a fairly small addition.
#20
I committed your patch. It will be available in the next -dev. In my opinion, it solves the problem. There are possible improvements in the UI (see #16) and the update procedure. Maybe we get that in the next stable release, but I'd recommend to file a new issue for the UI.
Feel free to re-open this issue, if you disagree.
#21
Whoop! First patch win! I might have a play with creating a centralised preference page at some point, as I can see that being more intuitive, and a job that's probably suited to my current level of expertise...
#22
I like the way, fields can be activated as Block at the moment. Maybe we can improve the documentation.
For first time users, it is much more intuitive to set the "enable cck_block" option at the fields preferences page. A centralised page is interesting for updaters and power users.
#23
Hey, I just realized that there is no chance to see, wheather a block is enabled for an instance or at the field level. Is there a fast way to change the name of the block to something like "CCK: FIELD_NAME (INSTANCE_NAME)" if it is only enabled for one instance? So you provide different blocks for each instance?
#24
Am I right in thinking you're suggesting separate blocks in the block admin page for each field instance, if enabled at the instance-level, instead of globally? Might that lead to the excess of blocks that caused this issue?
Or are you simply suggesting that if a block is enabled at the instance level, then it's informatively labelled with the instances is appears in?
Anothor option that might work:
- There is a centralised preference page for altering global field-level settings, with a note by each field listing any instance settings that override this global setting. This would allow for the listing in the blocks page to remain as it is, but should a user want an overview of settings, it would be easy to see. I considered a full on listing of all settings (all fields, with their settings for each instance) but realised the page would soon get unwieldy.
#25
You're right, my first suggestion aimed at one block for each instance of the field. But: I realized the implications of my post after a while and I don't see any use of one block per instance.
I think my mistake was a result of a slightly different meaning of the two settings for the blocks: using the field-level settings is just enabling or disabling the block for that field, whereas the instance-level settings are rather used as some visibility option. When looking at the blocks configuration page, there's no difference, whether one enables the block on the instance-level or on the field-level. As soon as one of the options is set to "enabled", the block shows up in the blocks list.
So what results from that knowledge?
1) the instance-level settings are a redundancy: enabling the block on the field-level and disabling it for one specific instance is like enabling the block and check the 'exclude' box at the field visibility settings for cck blocks. On the other hand, enabling the block only for one instance is the same like enabling the block on field-level and set it to 'exclude' or 'hidden' for all the instances you don't want to have this block shown up.
2) we can keep it the way, we have it with your patch or we can remove the redundancy by removing the instance-level settings. Then, some people have to do some mor brain work to built their site the way they want it, but other people don't get confused by two settings, producing the same result.
3) there's no need for one block per field instance.
4) if we provide the information about the block visibility on instance-level, we have to include the settings from the visibility setup page.
@donquixote: what do you think? You seem to be very interested in that topic and you seem to have an idea, how you expect that settings to work.
The only decision to make is whether we keep the field-instance setting or not. (the funny thing about that is, that I recommended only to have field specific settings some time ago *g*).
I'm setting this to 'fixed' again, as there are only few adjustments to make.
#26
Interesting question. This is what I think:
Separate blocks for each instance?
Nope. This would make things a lot more complicated and serve only a small minority of users. For the rare case where this is really needed, you can consider:
- making separate CCK fields.
- using the multiblock module
- using a computed_field or dynamic_field, to clone a field value and put it into a different block.
- writing your own module.
Availability setting per field instance?
On the blocks management page, the instance does not matter at all, if there is only one block per field. One block can either be shown on that page, or not shown.
Thus, I have the strong opinion that a per-instance setting for the availability on the blocks management page is not needed.
Please keep in mind, we are only trying to tidy up the blocks management page. Having a few unnecessary blocks showing up on that page is really nothing we need to worry about.
Visibility setting per instance / content type.
Now this is a different thing! It could make sense to have checkboxes on each individual block's configuration page, where you can set visibility by content type.
This setting might even be useful for other blocks as well, but it has a higher relevance for cck fields.
I am still undecided if I would prefer a submodule that would implement this setting only for cck fields, or an independent module that would give all blocks a per-content-type visibility setting. Anyway, this doesn't need to be decided in this issue.
#27
Thank's for your reply. I think, we allready have visibility settings per instance via the display settings for each field for each content typ. Just set a field in the display settings for cck blocks to 'exclude'.
@Steve: as you developped this patch, do you see any need for the availability settings per instance?
If not, we all agree, that we solve this issue by making all fields not available as block by default and let the user enable it on field level, if he needs a field to be available as block.
#28
Ah okay, if we already have per field display settings we can just remove the per-field settings. I'll have a look...
#29
Here's a patch, cleans up some indentation too.
#30
Seems to work just fine.
Can anyone else verify that?
#31
Please review this issue in the upcoming -dev release.
#32
As I see patch #29 already applied but there's some issues with it.
1)
if (arg(0) == 'node' && is_numeric(arg(1)) &&and latter if(!node_access("view", $node)){ is expensiveI replace this all mess with menu_get_object()
Because we display blocks only on node-page so menu_get_object() returns loaded node with access check already
2) There's no need to duplicate node_build_content() and produce notice with undefined $teaser and $page variables
3) Field title is required so no need to check it's availability before translation. Also I think block admin title should be translated too.
4) Querying {blocks} without placeholder is security risk.
5) module does not have uninstall function! Maybe it's better to file another issue but working with #735900: Deleting module's blocks when module is uninstalled it brings a lot of flotsam in database
#33
Forget to point about
<?phpif (isset($form_state['change_basic'])) {
?>
This setting should not be displayed when changing a basic field info.
Else take a look at screenshot
#34
#32 Are all issues with the module that should addressed in other issues, not here! I just snuck in some better indentation to the module in the patch in #29. Can you raise the comments in #32 as separate issues.
A fix for #33 is required in this issue however.
#35
I file a separate issues from #32
So only one change #33 is required
#36
Andy, thank's for your comments and the patch. Seems to work perfectly (basic information form is no longer altered but the global settings form still is).
#37
Fixed in the next -dev.
#38
Automatically closed -- issue fixed for 2 weeks with no activity.