backdrop-contrib / conditional_content_block

A module that let's you provide different content options for different roles.
GNU General Public License v2.0
0 stars 0 forks source link

Should I be storing content of block in config #2

Closed stpaultim closed 2 years ago

stpaultim commented 2 years ago

I copied the basic structure of this block from another custom block I found available on a BackdropCMS.org site. That module stored it's contents in config, but it did not have text fields for content.

What do folks think? Is this the right way to store the contents?

yorkshire-pudding commented 2 years ago

I'm torn on this one. It doesn't seem right to have content in config, but at the same time it could be overly complicated to create a content type that it draws from.

If it's just a basic text field, maybe config is the way to go, but the problem comes if the module grows to more complex content types. Do you see this sticking with just plain text or becoming broader?

stpaultim commented 2 years ago

Do you see this sticking with just plain text or becoming broader?

Actually, I'm using a filter-html text format for these blocks. Because, my specific use case was to add a button and I wanted to add a class through the UI. ;-)

I guess, I need to look closer at how the custom blocks in core store their content. This is obvious, but I haven't looked there yet.

I do plan to add an existing content option, which would allow a user to select a regular node instead of storing the content in the block. I like that option, but I think some site owners would just prefer to type it in the block.

While we call this content and it looks like content, an argument can be made that often a block like this is used for things that are borderline content/config (could go either way). My personal use case for this block is a way to add a button or instructions to the site that is different for anonymous users than it would be for authenticated users. This is really administrative content, not stuff that the site editor needs to worry about.

yorkshire-pudding commented 2 years ago

Looks like it should be ok to store in config: image

All you're doing different is having two fields.

docwilmot commented 2 years ago

Layout stores block settings (the content would be settings too in this case) automatically for each block instance, so you dont really need config as such. But of course any time you remove a block from a layout, all the settings go. So the only time your module needs to have its own config is if youve got some settings that must persist. For example if you want every block to be titled Tims great block you could either hard code that in the module code, or save it as a setting that each block would access.

For this module though, I dont see any settings that should be unique, right? So you dont need a config settings.

Then you dont need a hook_block_save(), and all content will available in hook_block_configure() as $settings.

stpaultim commented 2 years ago

Thanks again @docwilmot, you are correct. I was primarily using config to store data, because that's how it was done in the module I used for an example. HOWEVER, the module I was working from was doing something quite different and config was probably the right solution in that case, but not in my case.

Current version of module is not using config at all. I may reintroduce config as I find the appropriate use for it.

stpaultim commented 2 years ago

I think this issue is resolved. I think we're storing data now the way that core does it, which is good enough for me. ;-)